-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
Some scoring fixes #537
Some scoring fixes #537
Conversation
SA is now correctly lost, making a 5-star non-SA possible.
f7dc186
to
86094db
Compare
Getting a body spotted by a non-target loses SA, even if that NPC is later killed after becoming a target or through a KillContext 1 "kill"
Ugh, all the tests failing of course |
86094db
to
8a84010
Compare
(<MurderedBodySeenC2SEvent>event).Value.Witness, | ||
) | ||
if ( | ||
!(<MurderedBodySeenC2SEvent>event).Value.IsWitnessTarget |
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.
I'm confused. What does this property actually check for?
Based on it's name, I could infer a few possible meanings:
- Is the witness a target?
- Has the target (kill) been witnessed?
The only reason I ask is that in H2/3, you can commit an illegal action in front of a target, they spot you and become a witness, but then you kill them, and your SA comes back (as long as they didn't "tell anyone" - i.e. the orange doesn't spread).
This behavior is not the same in 2016, where if you are spotted, even if it's by your target, it's RIP SA.
This ties back to my original question, it's not clear what this property does. Can we add a docstring on the typedef with a brief explanation?
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 property is true if the witness is a target.
That means that with this PR, if any body is found by a non-target, you will lose SA and cannot regain it by killing that witness.
I'll add a docstring.
The 2016 logic should be unaffected by this change.
I think it is a bit clearer if you look at the changes per commit.
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.
LGTM, thanks for the clarification!
b26ca60
to
37383a9
Compare
37383a9
to
0d71a1e
Compare
Co-authored-by: Reece Dunham <[email protected]>
It turns out that Peacock's "no noticed kill" bonus logic was wrong. It checked if a
MurderedBodyFound
event happened at the same time as aKill
event, which sounds logical, but that's not how IOI does it.IOI just listens to the
Witnesses
event, which means that whenever you get "compromised", you lose the no noticed kill bonus.Yes, that means that you can lose the bonus without even killing anybody. Thanks IOI.
Anyways, I've also fixed some edge-cases relating to 5-star no-sa scores, and added the "providence member eliminated" actionreward for carpathian, which was not in peacock yet?
In fact, the
NonTargetKillsAllowed
contract metadata typedef was even wrong? Not sure how that happened.The "providence member eliminated" challenge is a bit odd to be in the global challenges I think, but I couldn't find a way to add it to the location challenge file without it appearing in the list of challenges in-game. (this is the only location-specific actionreward afaik)