Skip to content

Fix a bad comparison against Target.Invalid.#21742

Merged
pchote merged 1 commit intoOpenRA:bleedfrom
RoosterDragon:fix-target-compare
Feb 15, 2025
Merged

Fix a bad comparison against Target.Invalid.#21742
pchote merged 1 commit intoOpenRA:bleedfrom
RoosterDragon:fix-target-compare

Conversation

@RoosterDragon
Copy link
Copy Markdown
Member

Target.Invalid acts like a NaN, and will not compare equal with itself. Compare against the TargetType instead, which performs the intended comparison.

Target.Invalid acts like a NaN, and will not compare equal with itself. Compare against the TargetType instead, which performs the intended comparison.
Copy link
Copy Markdown
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

Not tested, but clearly correct.

@pchote
Copy link
Copy Markdown
Member

pchote commented Feb 11, 2025

related: #21736

@PunkPun PunkPun added this to the Next Release milestone Feb 11, 2025
@anvilvapre
Copy link
Copy Markdown
Contributor

Wouldn't i just have been a if (target != null) check? Invalid was not an instance but default/null?

@anvilvapre
Copy link
Copy Markdown
Contributor

Wouldn't i just have been a if (target != null) check? Invalid was not an instance but default/null?

Or perhaps elsewhere you also create an instance with type invalid.

@RoosterDragon
Copy link
Copy Markdown
Member Author

Target.Invalid is the default(Target) - it's the custom equality that means we need to check the type.

@anvilvapre
Copy link
Copy Markdown
Contributor

default

I see. Since it's a struct thereby a value type it is an instance and not a null pointer. Mixing languages.

@pchote pchote merged commit 639dc7c into OpenRA:bleed Feb 15, 2025
@pchote
Copy link
Copy Markdown
Member

pchote commented Feb 15, 2025

@RoosterDragon RoosterDragon deleted the fix-target-compare branch February 15, 2025 14:10
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.

4 participants