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

Reduce impact of max order length checks #20637

Merged
merged 1 commit into from Feb 14, 2023

Conversation

pchote
Copy link
Member

@pchote pchote commented Jan 25, 2023

This PR adds two one simple workarounds to reduce the chances of #19849 occurring.

The first commit raises the limit by a factor of 4 for MP games, and the second commit removes the limit completely for skirmish.

@pchote pchote added this to the Next release milestone Jan 25, 2023
OpenRA.Game/Server/Connection.cs Outdated Show resolved Hide resolved
OpenRA.Game/Server/Connection.cs Outdated Show resolved Hide resolved
@penev92
Copy link
Member

penev92 commented Jan 25, 2023

So semi-related: making 2k-3k-4k infantry (takes only a few minutes), selecting all and issuing attack orders in rapid succession reliably causes a connection drop followed by an NRE in our translation code because it tries to translate orderManager.ServerError, but that is somehow null. In single player!
I can confirm the SP-oriented changes here mask that nicely by not letting the connection drop.

P.S.: The NRE is caught and logged, so it's not the worst thing in the world, but the game does end because of the connection drop (on bleed). But with the changes here none of that happens.

@pchote
Copy link
Member Author

pchote commented Jan 25, 2023

Updated.

@penev92
Copy link
Member

penev92 commented Jan 25, 2023

Honestly I think it would be better to band-aid multiplayer than ignore it.

@penev92
Copy link
Member

penev92 commented Feb 14, 2023

Changelog

Commit on prep: 1d588f6

@pchote pchote deleted the order-length-limit branch August 24, 2023 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants