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

kvui: abstract away client tab additions #3950

Merged
merged 1 commit into from
Sep 22, 2024

Conversation

Silvris
Copy link
Collaborator

@Silvris Silvris commented Sep 16, 2024

What is this fixing or adding?

Intending for this to get in before #3934, this should allow clients to be cross-compatible between the two client versions. The main (currently-only) failure point for current clients on the KivyMD branch is the swap of GameManager.tabs and what types are accepted its children. As such, this abstracts away this behavior into GameManager itself such that clients need only call the function and provide the content for the tab.

How was this tested?

Loaded up Starcraft 2's client, ensured that tabs worked correctly.

If this makes graphical changes, please attach screenshots.

@github-actions github-actions bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Sep 16, 2024
Copy link
Collaborator

@qwint qwint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

purely a code review, everything looks good and this will help immensely when balancing apworlds (and PRs) to work for current kivy and the kivymd branch (and will be so nice to clean up code even without that)

@Silvris
Copy link
Collaborator Author

Silvris commented Sep 19, 2024

@Ziktofel @FlySniper

Copy link
Collaborator

@FlySniper FlySniper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran a brief test where I verified the visual information in the client and the information in the communication files. I also sent a victory file and a location file. Seems to be working.

@ScipioWright ScipioWright added waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Sep 22, 2024
@NewSoupVi NewSoupVi merged commit f7ec3d7 into ArchipelagoMW:main Sep 22, 2024
18 checks passed
AustinSumigray pushed a commit to AustinSumigray/Archipelago that referenced this pull request Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants