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

Record scrollbar movement in dialogs #662

Merged
merged 5 commits into from
Feb 27, 2025

Conversation

NQNStudios
Copy link
Collaborator

I forgot to stage one more replay system fix. This enables recording and replaying scrollbars in dialogxml scrollpanes.

@NQNStudios NQNStudios added bug replay Affects the replay system labels Feb 24, 2025
@CelticMinstrel
Copy link
Member

I have two general comments on this.

  1. If every control now stores its ID, I think there are a bunch of code paths that could be simplified, where the control map is searched to get the ID of self.
  2. I don't like that the scroll pane now requires an ID to function. I understand why it does (controls that don't specify an explicit Id gets a randomized ID, which won't work with replays), but can we come up with a different way to solve this?

@NQNStudios
Copy link
Collaborator Author

I think the ui's randomized ids are deterministic through a ui_rand object.

I added the id to this one because I had already recorded a replay where the id wasn't recorded, so I wanted to be able to edit the replay xml to retroactively set the name to something I knew it would be.

@NQNStudios
Copy link
Collaborator Author

@CelticMinstrel I removed the unnecessary 'name' attribute from about-boe and confirmed that a scrollpane without a name will record and replay correctly.

@NQNStudios
Copy link
Collaborator Author

NQNStudios commented Feb 27, 2025

@CelticMinstrel as you pointed out, controls in containers weren't getting their name fields set.

In order to do that while avoiding future merge conflicts, I cherry-picked my commit which makes containers resolve relative positioning in their elements.

@CelticMinstrel CelticMinstrel merged commit 1262451 into calref:master Feb 27, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug replay Affects the replay system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants