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
Add more RevealDisguiseOn types and unhardcode some events #13639
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The interfaces seem nice and I guess they will find more uses longterm even.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good otherwise.
if (cloak != null && cloak.Info.UncloakOn.HasFlag(UncloakType.Infiltrate)) | ||
cloak.Uncloak(); | ||
foreach (var ini in notifiers) | ||
ini.Infiltrating(self); | ||
|
||
foreach (var t in target.TraitsImplementing<INotifyInfiltrated>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a very similar interface so I suggest to either use that one or move the Infiltrating
check before OnInside
is triggered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is using the traits of the target
, while we want the traits of self
. Moving could work, but are you sure that we don't regress anything else then? (The uncloaking also happened after checking if the infiltration is valid.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see.
96b6eff
to
fa4548d
Compare
Rebased. |
if (cloak != null && cloak.Info.UncloakOn.HasFlag(UncloakType.Infiltrate)) | ||
cloak.Uncloak(); | ||
foreach (var ini in notifiers) | ||
ini.Infiltrating(self); | ||
|
||
foreach (var t in target.TraitsImplementing<INotifyInfiltrated>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see.
Events means things like
Infiltration
,Demolition
andUnloading
. That was hardcoded to only uncloak units before, but now calls an interface which can be easily implemented by custom traits.