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 casts of traits that are already known to be IDisabledTraits #20361

Merged
merged 3 commits into from Oct 14, 2022

Conversation

abcdefg30
Copy link
Member

Exts.IsTraitEnabled(this T trait) uses trait is IDisabledTrait disabledTrait (and thus FirstEnabledTraitOrDefault does as well). That is not necessary if we already know that T is an IDisabledTrait.

I introduced a new FirstEnabledConditionalTraitOrDefault that directly checks where T : IDisabledTrait in the signature and replaced unnecessary usages of Exts.IsTraitEnabled with direct calls to IsTraitDisabled.

@anvilvapre
Copy link
Contributor

anvilvapre commented Oct 14, 2022

I did a separate experiment in the past to determine the impact of the cast.

I tried to Func<T extends ConditionalTrait>() also avoiding the need to cast, but it did not give an obvious performance benifit.

I would be in favor of a trait base class that would have a flags fields that would indicate for any trait if it was enabled, paused perhaps timed. For non conditional traits it would always be true.

Also because more and more traits seem to become conditional, because the game developers want more flexibility to enable traits.

@PunkPun PunkPun merged commit 6e6c828 into OpenRA:bleed Oct 14, 2022
@PunkPun
Copy link
Member

PunkPun commented Oct 14, 2022

Changelog

@abcdefg30 abcdefg30 deleted the disabledTrait branch October 14, 2022 11:13
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