Skip to content

Sanitize saved skirmish factions. - #21756

Merged
PunkPun merged 1 commit into
OpenRA:bleedfrom
pchote:fix-skirmish-options-faction
Feb 22, 2025
Merged

PunkPun merged 1 commit into
OpenRA:bleedfrom
pchote:fix-skirmish-options-faction

Conversation

@pchote

@pchote pchote commented Feb 22, 2025

Copy link
Copy Markdown
Member

Fixes a crash reported on Discord.

Testcase: edit the skirmish.{mod}.yaml file and set bogus values. The existing logic already handled bogus options, but no checking was done for faction names.

@pchote pchote added this to the Next Release milestone Feb 22, 2025
Comment thread OpenRA.Mods.Common/ServerTraits/SkirmishLogic.cs Outdated
@pchote

pchote commented Feb 22, 2025

Copy link
Copy Markdown
Member Author

Updated.

if (!isValidFaction)
{
server.SendFluentMessageTo(conn, InvalidFactionSelected, new object[] { "faction", faction });
server.SendFluentMessageTo(conn, SupportedFactions, new object[] { "factions", factions.JoinWith(", ") });

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't report these for any of the other options, and this info isn't very useful for the client - so better removed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sence

Comment thread OpenRA.Mods.Common/ServerTraits/SkirmishLogic.cs Outdated
@pchote
pchote force-pushed the fix-skirmish-options-faction branch from 861fa5c to 376dbaa Compare February 22, 2025 12:57
@pchote

pchote commented Feb 22, 2025

Copy link
Copy Markdown
Member Author

Reworked to handle bots and reduce duplication.

@PunkPun
PunkPun merged commit 541cf3b into OpenRA:bleed Feb 22, 2025
@PunkPun

PunkPun commented Feb 22, 2025

Copy link
Copy Markdown
Member

@pchote
pchote deleted the fix-skirmish-options-faction branch February 22, 2025 13:38
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.

3 participants