Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve how menus are displayed with invisible > indicators #5

Merged
merged 2 commits into from
Dec 5, 2021

Conversation

curiousdannii
Copy link
Contributor

@curiousdannii curiousdannii commented Nov 29, 2021

TADS menus print a > at the beginning of each line, rather than a space.
It is not visible however because the colours are set to modes that are meant to make it transparent.
Without measuring colours (as it is not yet possible with Remglk/GlkOte) we can instead replace the invisible text with spaces.
But this is not perfect as the width of one proportional width > and one fix-width space may not be the same.

@curiousdannii
Copy link
Contributor Author

Maybe it would be better to do this only if measuring doesn't work? But is it worth that complexity, just for 1-2 pixels of misaligned text?

@angstsmurf
Copy link
Contributor

Maybe it would be better to do this only if measuring doesn't work? But is it worth that complexity, just for 1-2 pixels of misaligned text?

I'd say it's worth a bit of extra complexity to make the text line up nicely.

@curiousdannii
Copy link
Contributor Author

Okay. I'll probably rework this to be based off your earlier method, and then use the replacing-with-spaces only if measuring doesn't work.

angstsmurf and others added 2 commits December 1, 2021 12:44
Create an invisible style and use it when text color
is OS_COLOR_P_TEXTBG.
TADS menus print a > at the beginning of each line, rather than a space.
It is not visible however because the colours are set to modes that are meant to make it transparent.
Prior to now the Glk port did not handle these colour modes.
First we include angstsmurf's patch from Spatterlight to add stylehints for transparent text.
But this won't work if we can't measure colours (as is currently the case for Remglk, and may always be.)
So instead we can replace the invisible text with spaces.
This is not perfect as the width of one > and one fix-width space may not be the same, but it's hopefully very close.
@curiousdannii
Copy link
Contributor Author

curiousdannii commented Dec 1, 2021

@angstsmurf I switched to your patch, and then built my method on top.

Though this probably really needs to have the rest of the colour changes applied too.

Edit: this PR now includes #4 as well

@curiousdannii
Copy link
Contributor Author

@angstsmurf Did you have the chance to test this?

@angstsmurf
Copy link
Contributor

I tried it out in Gargoyle (on macOS ARM) on Thaumistry: In Charm's Way and The Devil in the Details, and it looks and work great. Much nicer than the old Gargoyle way to display the TADS 3 menus.

Let me know if there is anything in particular you want me to try. I have not been able to test this with Spatterlight yet.

@cspiegel
Copy link
Contributor

cspiegel commented Dec 4, 2021

I've tested this against Gargoyle as well. I use white on grey for my default Gargoyle colors, and that seems to cause some misbehavior with this patch.

Here's how things look with the current master of this repository playing Dark Angel:

Screenshot_20211204_102414
Screenshot_20211204_102438

When I apply the changes in this pull request:

Screenshot_20211204_102513
Screenshot_20211204_102526

I just tested with a white background and it's also not ideal:

Original:

Screenshot_20211204_102707

Patched:

Screenshot_20211204_102731

For reference, here's what QTads looks like with the same color scheme as Gargoyle's white on grey:

Screenshot_20211204_102820

@curiousdannii
Copy link
Contributor Author

curiousdannii commented Dec 4, 2021

Well the main thing this PR is meant to do is fix the multiple >s being shown in menus, and it seems like that is working. We have more work to do to improve the general styling and colour support.

But considering many games have weird looking styles even in qtads I'm not sure how much we can do.

In addition to fixing the > problem, this PR also switches to real reverse stylehints for reverse mode, and also temporarily disables os_banner_set_screen_color. I assume its that last thing that is causing issues.

I think I'll merge this now, cause I expect to have time to work on the other colour functions later today. So hopefully they'll be much improved too!

@curiousdannii
Copy link
Contributor Author

curiousdannii commented Dec 5, 2021

@cspiegel Actually, thinking again, I realised it was a mistake to put the third commit into this PR. I wanted it for Parchment, and was lazy and included it here, when I should've made a separate branch for Parchment to reference. Sorry for that.

I'll take out the last commit. The other two should be a strict improvement over what Gargoyle currently has.

@curiousdannii curiousdannii merged commit a5d614e into master Dec 5, 2021
@curiousdannii curiousdannii deleted the invisible-menus branch December 5, 2021 01:53
@realnc
Copy link
Member

realnc commented Dec 5, 2021

Dark Angel forces a white background for some of the text it prints. So these screenshots are all correct, actually. This is a bug in the game. It looks like that even with htmltads.exe from the Windows TADS Player Kit.

What the game should be doing is use parameterized background colors (like bgcolor or statusbgcolor), which are intended to match the current background color, whatever it happens to be. But instead, it uses white. So these results are expected and correct.

@cspiegel
Copy link
Contributor

cspiegel commented Dec 5, 2021

Dark Angel forces a white background for some of the text it prints. So these screenshots are all correct, actually. This is a bug in the game. It looks like that even with htmltads.exe from the Windows TADS Player Kit.

The issue I have is that the patches vs unpatched versions differ. The white background in the text does look wrong, but shouldn't it be wrong consistently? Dannii notes that there are upcoming color fixes so it may be moot, but I can't see how all screenshots can be correct when some contradict each other.

In short, the status bar is "properly" reverse video in the unpatched version, but not reverse in the patched. The forced-white background is a red herring here, as it's the general look of the status window that looks wrong.

I'm assuming the reverse-video status bar is right, given that QTads does it.

@realnc
Copy link
Member

realnc commented Dec 5, 2021

I'm assuming the reverse-video status bar is right, given that QTads does it.

These are supposed to be user preferences. The standard Tads game library (adv/adv3) doesn't actually try to force any kind of color for banners. The interpreter is free to choose whatever color it wants, both for banners as well as the main window. QTads just uses the same text color for banners and the main window, and a gray background for banners. Even though this isn't a completely arbitrary choice (it just copied what htmltads.exe does), it's perfectly legal for a terp to choose different default colors and allow users to change them.

If a game wants its status bar (or any other banner window it creates) to have a specific background color, it must do so explicitly.

@curiousdannii
Copy link
Contributor Author

@cspiegel The changes to window styles were an extra commit that is no longer part of this PR, and wasn't merged into master. I'll keep developing the style changes and present a PR that is a full upgrade.

We do definitely need some style changes though, as #3 is a major bug. In #7 I proposed that all colour support in TADS moving forward switch to using the garglk extension functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants