-
Notifications
You must be signed in to change notification settings - Fork 53
feat(kswole.lic): add FORCERT for shield mind #1957
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
Conversation
Whirlin suggested that shield mind could be executed during rt and he was correct after testing. Given the nature of the ability, we would always want to execute the command during round time to avoid taking a big hit from a spell. Removed pausing a script as it is no longer needed. Removed waitrt? and waitcastrt? as it is no longer needed.
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.
Important
Looks good to me! 👍
Reviewed everything up to 0a7002a in 1 minute and 39 seconds. Click for details.
- Reviewed
35lines of code in1files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. scripts/kswole.lic:253
- Draft comment:
Removed waitrt?/waitcastrt? and pause/unpause calls. Confirm that timing issues are handled by 'forcert' (e.g. edge cases)? - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that timing issues are handled byforcert, which is a request for confirmation of intention. This violates the rule against asking the PR author to confirm their intention or ensure behavior is intended. Therefore, this comment should be removed.
2. scripts/kswole.lic:256
- Draft comment:
Update changed command string label to 'shield mind forcert' consistently in logging/changelog. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. scripts/kswole.lic:19
- Draft comment:
Updated metadata: The 'tags' now include 'shield mind', which matches the new functionality. Ensure that all documentation and tag-based filters in the system are updated accordingly. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. scripts/kswole.lic:253
- Draft comment:
Shield Mind update: Removed wait and script pause calls and now using 'shield mind forcert'. Confirm that eliminating these pauses does not introduce race conditions with concurrently running scripts. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that removing pauses does not introduce race conditions. This is a request for confirmation of behavior, which violates the rule against asking the author to confirm their intention or ensure behavior is intended.
5. scripts/kswole.lic:30
- Draft comment:
The changelog and runtime command use 'shield mind forcert'. Please double-check if 'forcert' is the intended term or if it should be 'force' (or another variant), to avoid confusion. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment asks for verification about terminology, which violates our rule about not asking authors to verify/confirm things. The code and changelog are consistent in using 'forcert'. The changelog even explains the purpose - to execute during roundtime. This seems to be an intentional choice, not a typo. Maybe 'forcert' is actually incorrect terminology in the game system and this comment is trying to prevent a real issue? Even if 'forcert' was incorrect, asking for verification is not the right approach - the comment should definitively state the correct term if there is one. More importantly, we don't have strong evidence that 'forcert' is wrong. Delete this comment. It violates our rule about asking for verification, and the code/changelog consistently use 'forcert' in an apparently intentional way.
6. scripts/kswole.lic:206
- Draft comment:
In the regex patterns for Forgotten Vineyard, the term 'bardesss' appears. This extra 's' may be a typographical error. Consider reviewing if it should be 'bardess' or left as is if intentional. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_4no5OKzKAtomf3n0
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
| Version Control: | ||
| Major_change.feature_addition.bugfix | ||
| v1.1.0 (2025-05-08) |
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.
Version not bumped proper. This should be v1.1.1 and need to update the actual version above it as well.
| @scripts_to_pause.each { |script| Script.pause(script) if Script.running?(script) } | ||
| shieldmind = dothistimeout('shield mind', 2, /thereby forcing any incoming attacks against your mind or soul to penetrate your|You must be wielding a shield|^You cannot muster the necessary focus to shield your mind and soul quite so soon\./) | ||
| @scripts_to_pause.each { |script| Script.unpause(script) if Script.running?(script) } | ||
| shieldmind = dothistimeout('shield mind forcert', 2, /thereby forcing any incoming attacks against your mind or soul to penetrate your|You must be wielding a shield|^You cannot muster the necessary focus to shield your mind and soul quite so soon\./) |
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 increases the cost of the PSM dramatically and is not factored in the current activation usage scenario. Is there a way to know how many activations you've done currently so you can estimate the stamina usage? Also what happens if you try to use FORCERT without any MOC ranks at all? Does it error? Does it not work while in RT? What happens if you don't have enough stamina, will it just fail to work? Or will it blow your muscles?
Alternatively, end-users could just toggle the usage of FORCERT for their shield skills globally?
|
Any additional updates for this to be inclusive of the previous comments? |
Whirlin suggested that shield mind could be executed during rt and he was correct after testing. Given the nature of the ability, we would always want to execute the command during round time to avoid taking a big hit from a spell.
Removed pausing a script as it is no longer needed. Removed waitrt? and waitcastrt? as it is no longer needed.
Important
Update
kswole.licto executeshield mindwithforcertduring round time, removing the need for script pauses and wait times.shield mindwithforcertduring round time inkswole.lic.waitrt?and script pause forshield mindexecution.1.1.0with changes toshield mindexecution.shield mindto tags inkswole.lic.This description was created by
for 0a7002a. You can customize this summary. It will automatically update as commits are pushed.