Adding new world event guiSelectedRowChanged #527
Conversation
|
Actually, I think the event should probably be called “guiSelectedRowChanged” to be a little clearer about what is happening. What do you think? |
|
Yeah, that's probably a more descriptive name.
…On Thu, Aug 28, 2025, 15:54 phkb ***@***.***> wrote:
*phkb* left a comment (OoliteProject/oolite#527)
<#527 (comment)>
Actually, I think the event should probably be called
“guiSelectedRowChanged” to be a little clearer about what is happening.
What do you think?
—
Reply to this email directly, view it on GitHub
<#527 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEYHJTT4N73DCNVWPOO6DL3P33XRAVCNFSM6AAAAACFAHTOMGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEMZTGM4DOOJYGQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
phkb
changed the title
|
Also updated the initial description with the new event name and added some additional info about the parameter types. |
|
I noticed that when guiSelectedRowChanged fired, sometimes guiScreen wasn't updated to the correct screen. To correct this, I moved the update of guiScreen to be at the beginning of each screen build process, rather than at the end. Or at least before the first setSelectedRow call. |
|
This last change bringing the gui screen updates to the top triggers my alarms for some reason. Haven't had more than a cursory glance, but are we sure that the from and to gui screen parameters are not affected by it? |
|
I checked every from/to combination I could find, for the ones where I made a change, and couldn't find any instance of where guiScreenChanged reported the change incorrectly. There was some odd code I didn't understand, though: in a couple of instances, this code line was set before and after the guiScreen value was changed: Or did your alarm bells ring for another aspect of gui screen changes (background, overlays, something else)? |
|
My alarms went off for the gui screen changes initially, but the items you
mentioned are also valid potential fail points that need to be checked.
About the [self setDemoShips:NO];, it might just set a flag, but maybe
further down a method is called that uses the current state of demoShips in
combination with the current gui screen. Haven't looked at the code and I
get possibly paranoid but the point is that just because a flag is changed
without any immediate effects doesn't mean that everything can be
considered safe.
If you are confident that all bases are covered and the risk of moving the
gui screen changes to the top is negligible, we are good to go. Just wanted
to indicate potential points of concern.
…On Sun, Aug 31, 2025, 03:55 phkb ***@***.***> wrote:
*phkb* left a comment (OoliteProject/oolite#527)
<#527 (comment)>
I checked every from/to combination I could find, for the ones where I
made a change, and couldn't find any instance of where guiScreenChanged
reported the change incorrectly. There was some odd code I didn't
understand, though: in a couple of instances, this code line was set before
and after the guiScreen value was changed:
[self setShowDemoShips:NO];
But it's just setting a flag, and guiScreen is just a value; neither of
them execute additional code when the value changes, do they?
Or did your alarm bells ring for another aspect of gui screen changes
(background, overlays, something else)?
—
Reply to this email directly, view it on GitHub
<#527 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEYHJUXDTCD4SIE56D63R33QJBX7AVCNFSM6AAAAACFAHTOMGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEMZZGY2DANJQGM>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
To be clear, not every screen had the issue. It was only a couple of them where the change was happening at the end of the build process. But the reminder of being cautious is appreciated. I’ll spend tomorrow looking through the code and running some more tests to see if anything shows up. |
|
I've examined the code again, looking at all the function calls between the two points of the gui screens where the change took place: from where I've moved the change to, and to where it was previously, and can't find anything that was dependent on having a specific guiScreen value. I've also tested a few OXP's that depend on having specific values for guiScreen (the "Red Set", Xenon UI, BGS in particular), and they all seem to function correctly. I honestly think I've exhausted all the ways the code could break and haven't found any indication that might be happening, so I think we're good. But I'll leave this until tonight in case you have any additional thoughts. |
|
Looks like we are good to go then. Feel free to merge when ready. |
This PR adds a new world event, "guiSelectedRowChanged", which is raised any time the player selects a new item on a gui screen. This could happen by using the up and down arrow keys, or by using the mouse.
The goal of this event is to give scripters the ability to see what the player is doing on a screen and respond to it before the player presses the enter key. For instance, looking towards accessibility development, the text of the selected item could be read out to the player audibly as soon as they select it.
The format of the event is:
this.guiSelectedRowChanged = function(key, row, text) { }Where
key (string) = the underlying key code for the selected row, based on the screen currently shown.
row (int) = the gui screen row index value for the selected row
text (string) = the text of the selected row
The event will fire on all screens that have any sort of gui element, including the options screen (F2), the game options screen, and any mission screen. Screens where the event doesn't fire are the various chart screens (F6 and F6F6), the system info screen (F7), the manifest screen (F5F5), but only if there is a single page, and the market info screen (F8F8).