-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Mind Control Threshold #627
Mind Control Threshold #627
Conversation
Now MC weapons can replace the Mind Control stuff by alternative warheads if the enemy health percentage is over the MindControl.threshold value.
Nightly build for this pull request:
|
Can't such functionality be implemented using |
MindControl is special logic that doesn't quite work through the critical hit system. The crit warhead wouldn't count as mind control warhead and the unit wouldn't have the CaptureManagerClass set up or the MC pips if it is its only MC weapon either. Also there's no need to reinvent the wheel with the odd percentage parsing and health calculations. INI parser happily parses percentages to Use Having the threshold without having to set the Damage and Warhead could also be desirable. |
Perhaps having an option to invert threshold would be good? I already see two usecases:
|
The only reason to reinvent the wheel was that in the first implementations that method returned unrealistic huge numbers so I made calculations by hand as you can see.
Phobos develop branch doesn't use this method so the only 2 valid examples are located in Ares sources... I'll try later (because ask for values I never used like land Type etc) but the current animation code is C&P from Crit system so if is incomplete here the same there.
I don't understand this, can you explain it, please? |
Yes, the 2 user cases you mentioned are covered with the current tags. I had in mind the multi-engineer logic from RA1. I don't know if that is useful. Inverting the threshold means:
Maybe a 'healer' that can MC units if the HP % is over the threshold? |
due to feedback.
Applied feedback with those 2 internal changes. |
I forgot to replace 1 line of code
Due a change in YRpp
0fa5476
to
da80463
Compare
MindControl.Threshold.Inverse=bool Instead of damage if enemy HP percentage is over the threshold if the tag is "true" the comparison is flipped (over the threshold Mind-Control and below the threshold damage the mind-controllable target). MindControl.Damage now is called MindControl.AlternativeDamage MindControl.Warhead now is called MindControl.AlternativeWarhead
Max Lenarc here, just reporting my attempt to test the Mind Control Threshold from a while ago since I have not posted here in Github. I doubt it will be useful though. Ultimately failed to get the feature to work, not sure why though. I have the latest nightly build with the code given to me on Mod Haven. I think it may be a case of the code being put in the wrong spot in the INI? Versus.mcvheavy=0% Versus.t2mediump=0% Versus.t2heavyp=0% Versus.t3heavyp=0% Versus.mindarm=0% Transact=true MindControl.Threshold= 25% ; positive percentage. Represents a percentage from 0% to 100% [PsychoFire] |
There are some errors in the documentation. It says "AlternativeDamage/Warhead" but the actual tags are "AlternateDamage/Warhead". |
Updated the tag names in the documentation
I updated the tag names in the docs with your feedback. |
The problem was the tag names, I updated the docs. |
So that was the problem. Thanks for the info! |
Try it so we can label it as tested |
Got to testing it again. Everyone works as intended. Admittedly I misunderstood what MindControl.Threshold.Inverse did at first; I thought it killed Mind Controlled units under the Threshold, not kill unit enemy units under the Threshold. I think it is good to go. One of the coolest logics I've seen here! |
Nightly build for this pull request:
This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build. |
Warning Rate Limit Exceeded@FS-21 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 28 minutes and 45 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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 entire implementation could be generalized and massively simplified. There is nothing about this feature that would inherently require tying it to MindControl
. In addition the current implementation reimplements a lot of things that it really does not need to.
In fact, because we now have ExtraWarheads
this feature can be simplified down a lot. You can generalize the threshold system to not be tied to MindControl
, remove all of the unnecessary (nullptr) checks that just bloat and slow down the code - only thing you realistically need to check is that the bullet's target is instance of ObjectClass
or derived classes, e.g it has health. There's no need for damage, there's no need to Warheads or kill switches. The only thing needed is skipping Warhead detonation if the threshold check passees. Because we have ExtraWarheads
, you can combine two or more Warheads to create health-based conditional Warhead detonation this way without requiring any other bespoke code.
Only issue is that swapping from regular -> MindControl
via ExtraWarheads
won't work due to absence of CaptureManagerClass
but that could hypothetically be fixed. And it is not like this feature's original scope implemented that scenario either.
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.
EDIT: missed that Starkku has already posted the essay, I basically posted more or less the same but with more details and suggestions/explanations. Hopefully would still be useful.
I think the way this feature is designed is too specific. IMO the best would be reworking it into generic threshold logic.
After some brainstorming with @Starkku we came to conclusion that this whole logic can be made much simpler and much more usable. Per idea you would only implement two tags:
[SOMEWARHEAD]
AffectsBelowPercent=100% ; means HP of the target
AffectsAbovePercent=0% ; means HP of the target
(both conditions must be fulfilled for the warhead to activate so it works like a range, for example you could have a range between 20% and 80% percents. The code would be just a two-line check at the very beginning of BulletClass::Logics
that would bail from func if the conditions pass)
Why? Because we should be doing features that mesh with other features like bricks in the wall, and not overly specific and hardly reusable ones. This way every small feature is simple, and together the features allow for great flexibility.
As such, if you combine it with Crit
or ExtraWarhead
logics you can do an arbitrary amount of warheads activating at arbitrary HP ranges. CanKill
can be done with relative damage logic, or if needed it could be implemented as a separate feature.
As for the mind control itself it only works if CaptureManagerClass
is inited for the firer, which currently only happens if one of the weapons is MC one, so to fix that you could iterate all the weapons with their normal, extra and crit (and other possible) warheads that the unit has on init (how the game does it, won't work in edge cases like interceptor replcaing WH with a MC WH), or check for CaptureManagerClass
when the MC occurs, and if not inited and the firer is there - init it. Or both, which would probably be the best.
```ini | ||
MindControl.Threshold=100% or 1.0 ; positive percentage. Represents a percentage from 0% to 100% |
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.
Missing section
Due to the well explained feedback is much better if I close this PR and I rewrite the feature from 0 following the suggestions. When I arrive home I'll close this. |
Well you can simply reuse this PR and just re-code, this shouldn't take much time or changes. |
Because I need it for releasing my next mod release I'll update with latest develop commit and then mark it as "draft" because I need to rewrite all the code. |
I'll close this PR for the time being, let's move onto the #1398. |
Now MC weapons can replace the Mind Control stuff by alternative warheads if the enemy health percentage is over the MindControl.threshold value.
More information in the documentation.