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 the server loading and the client receiving invalid requirements #2086

Merged
merged 2 commits into from
Dec 26, 2023

Conversation

lmoureaux
Copy link
Contributor

This makes the server fail to load the ruleset when an invalid enum-based requirement is specified. The LT79 and LeagueB2 rulesets have one and will stop loading. Since this is the server doing more error checking instead of ignoring invalid values, this is acceptable. We should remove the offending reqs from the games repo.

A client counterpart of the server-side checks is also implemented (duplicating a lot of code in the process...), so the client can now detect when the server sends invalid requirements. It prints a warning and sets the requirement to an invalid value that it won't attempt to interpret later on. This allows forward compatibility with future server versions.

Tested by trying to load the LT79 ruleset, loading the classic ruleset successfully, and connecting to broken LeagueB2 where the server has loaded a couple invalid requirements.

Closes #2032.

Check that the value of enum-based requirements is valid and fail otherwise.

See longturn#2032.
Checks are copied from universal_value_from_str().

We don't fail when encountering invalid requirements because we want to keep
forward compatibility and new requirement types are added often. Instead we set
them to the "none" requirement so we don't try to interpret them.

Closes longturn#2032.
@lmoureaux lmoureaux requested a review from jwrober December 25, 2023 04:09
Copy link
Collaborator

@jwrober jwrober left a comment

Choose a reason for hiding this comment

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

LGTM

@lmoureaux lmoureaux merged commit 434d7ef into longturn:master Dec 26, 2023
19 checks passed
@lmoureaux lmoureaux deleted the bugfix/invalid-enums branch December 26, 2023 01:28
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.

[LT79] Assertion nullptr != src failed
2 participants