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

Fix world closing in client gametests #4327

Merged
merged 1 commit into from
Dec 26, 2024

Conversation

Earthcomputer
Copy link
Contributor

There was an issue where the assertion that the game is on the title screen would fail when ending a test by closing a singleplayer world (via a try-with-resources close of TestSingleplayerContext).

This was caused by the threading system deferring the client.disconnect call until after the client thread task wait loop (i.e. until the gametest thread waits a tick). Therefore, the order of operations inside GameMenuScreen.disconnect was reversed and the title screen was shown first before disconnecting. Moreover, the title screen was visible to the gametest thread until ticks were waited, which allowed context.waitForScreen(TitleScreen.class) to return immediately after waiting 0 ticks.

Rather than relying on navigating the game menu which may have been changed by mods, I thought it was better to refactor the code to disconnect directly, and I've written it in a way which waits for the world to be null directly after calling client.disconnect, which allows for the deferred call to run.

Copy link
Contributor

@kevinthegreat1 kevinthegreat1 left a comment

Choose a reason for hiding this comment

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

Works for me. (Tested by manually using this version.)

@modmuss50 modmuss50 merged commit cdad5be into FabricMC:1.21.4 Dec 26, 2024
4 checks passed
@Earthcomputer Earthcomputer deleted the fix-world-closing branch December 26, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants