Skip to content

Conversation

@Deysh
Copy link
Contributor

@Deysh Deysh commented Nov 30, 2025

Important

This PR adds boon creature support to the Bigshot script, refactors command logic, and cleans up code for improved functionality and readability.

  • Boon Creature Support:
    • Added boon creature handling with new methods check_boons, invalid_target_with_boons, and should_flee_from_boons? in bigshot.lic.
    • Introduced boon-related settings and logic in refresh_boon_settings and update_boon.
    • Added boon abilities initialization in initialize_boon_data.
  • Code Cleanup:
    • Removed redundant JOIN_LEADER event and replaced with FOLLOW_NOW in multiple places.
    • Refactored command_check to use regex and lambdas for cleaner logic.
    • Simplified find_routine and ma_looter logic.
  • Miscellaneous:
    • Updated debug messages to include caller information.
    • Adjusted GUI elements for boon creature settings.
    • Fixed minor typos and improved comments for clarity.

This description was created by Ellipsis for b0a6d4e. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed everything up to f20243f in 6 minutes and 3 seconds. Click for details.
  • Reviewed 1000 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 18 draft 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/bigshot.lic:1740
  • Draft comment:
    In update_boon, the @updating flag is manually set/reset. To avoid leaving @updating true on exceptions, consider wrapping the critical section in a begin…ensure block so @updating is always reset.
  • 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.
2. scripts/bigshot.lic:2232
  • Draft comment:
    The BOON_ABILITIES hash uses downcased adjectives but the user‐supplied boons (BOONS_IGNORE and BOONS_FLEE) are taken directly from UserVars. Normalize these (e.g. downcase) to avoid mismatches due to case differences.
  • 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.
3. scripts/bigshot.lic:6190
  • Draft comment:
    The tooltip key in invalid_target_with_boons contains a typo ('was are ignoring'). Consider rewording it to something like 'target has boons that are to be ignored'.
  • 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.
4. scripts/bigshot.lic:1730
  • Draft comment:
    The check_group method toggles off both the header and its sub-items when all checkboxes are active. Confirm that this behavior is intentional, as it may be counterintuitive for users expecting a sticky ‘all’ selection.
  • 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.
5. scripts/bigshot.lic:3730
  • Draft comment:
    Use of 'loop do' (instead of braces) for multi-line loops improves readability. This change is stylistic but consider enforcing consistent style across the project.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
6. scripts/bigshot.lic:7620
  • Draft comment:
    In the tail follow block, the leader’s target is resynced if available. Ensure that group.leader_target returns a valid, non-nil target before overriding the current target to avoid unintended nil assignments.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
7. scripts/bigshot.lic:1023
  • Draft comment:
    Typographical error: 'group_deader' appears to be misspelled. Did you mean 'group_leader'?
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
8. scripts/bigshot.lic:1112
  • Draft comment:
    Typo: In the tooltip text property, "Saves setting changes to the current profile." might be intended as "Saves settings changes to the current profile." (i.e. adding an 's' to 'setting').
  • Reason this comment was not posted:
    Comment was on unchanged code.
9. scripts/bigshot.lic:1121
  • Draft comment:
    Typographical note: The text "Afk scripting on any server but Shattered is in violation of game policy." uses "Afk". Consider changing it to "AFK" for consistency with common acronym capitalization.
  • Reason this comment was not posted:
    Comment was on unchanged code.
10. scripts/bigshot.lic:1141
  • Draft comment:
    There is an extra leading space in the label text " Fog ONLY if wounded or encumbered". Consider removing the leading space for consistency.
  • Reason this comment was not posted:
    Comment was on unchanged code.
11. scripts/bigshot.lic:1142
  • Draft comment:
    There is an extra leading space in the label text " Fog twice if returning from the rift". Consider removing the leading space for consistency.
  • 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.
12. scripts/bigshot.lic:1190
  • Draft comment:
    Typo: "sensative" should be corrected to "sensitive".
  • Reason this comment was not posted:
    Comment was on unchanged code.
13. scripts/bigshot.lic:1400
  • Draft comment:
    Typographical error: The id 'group_deader' in the GtkCheckButton may be a typo. Consider correcting it (e.g., 'group_dead' or a more appropriate name) to match its label 'Stop for dead group members'.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
14. scripts/bigshot.lic:1985
  • Draft comment:
    Typo found in the comment: "turn everyting off" should be "turn everything off".
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a legitimate typo in a comment. However, I need to consider whether typos in comments are important enough to warrant PR review comments. The rules state "Do NOT comment unless there is clearly a code change required" and "Do NOT make comments that are obvious or unimportant." A typo in a comment doesn't affect functionality and is relatively minor. While fixing typos improves code quality, this seems like a very minor issue that might not meet the threshold for a PR comment, especially since it's just a comment and not actual code. The typo is real and fixing it would improve code quality. Comments should be clear and professional. However, this is an extremely minor issue that doesn't affect functionality at all. The rules emphasize not making obvious or unimportant comments, and a single-letter typo in a comment could be considered unimportant. While the typo is minor, having correct spelling in comments is part of code quality and professionalism. However, given the rules explicitly state not to make "obvious or unimportant" comments, and this is just a typo in a comment (not code), this likely falls below the threshold for what should be flagged in a PR review. The author could fix this if they notice it, but it's not worth a review comment. This comment should be deleted. While the typo is real, it's too minor and unimportant to warrant a PR review comment. It's just a spelling error in a comment that doesn't affect functionality.
15. scripts/bigshot.lic:2494
  • Draft comment:
    Typo suggestion: In the 'boosted_defense' entry, the adjective 'flexile' might be a typo for 'flexible'. Please verify if 'flexible' was intended.
  • 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% This comment is speculative ("might be a typo") and asks the author to verify rather than pointing out a definite issue. The word 'flexile' is a real English word, albeit archaic. In the context of a fantasy game (Gemstone), using archaic or unusual adjectives for creature descriptions is quite common and likely intentional. The comment violates the rule about not making speculative comments or asking the author to confirm/verify things. This is exactly the type of "please verify" comment that should be deleted. However, if 'flexile' truly is a typo and the author meant 'flexible', this could be a legitimate catch. Game data typos can be important if they affect gameplay or user-facing text. But without knowing the game's lore or seeing other similar descriptors, I can't be certain. Even if there's a small chance this is a typo, the comment explicitly asks the author to "verify" which directly violates the rules. The rules state: "Do NOT ask the PR author to confirm their intention, to explain, to double-check things, to ensure the behavior is intended". This is a textbook example of that pattern. Additionally, 'flexile' is a real word that makes sense in context. This comment should be deleted. It's speculative, asks the author to verify, and 'flexile' is a legitimate English word that fits the fantasy game context. The comment violates the rule against asking authors to confirm intentions.
16. scripts/bigshot.lic:2774
  • Draft comment:
    Typo suggestion: The key 'group_deader' seems off. It doesn't follow the naming pattern of similar keys and the comment implies "stop for dead followers". Please verify if 'group_deader' is the intended name or if it should be modified (e.g., 'group_dead') for clarity.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
17. scripts/bigshot.lic:5739
  • Draft comment:
    Typo in comment: "tails wont have followers" should be "tails won't have followers".
  • 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.
18. scripts/bigshot.lic:5919
  • Draft comment:
    Typo: The message string contains four periods ('Waiting for followers....'). Consider using an ellipsis (three periods) to match common punctuation conventions.
  • 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% This is a very minor stylistic issue about punctuation. While technically correct that standard ellipses use three periods, this is: 1) Not a functional bug, 2) Extremely minor and subjective, 3) The kind of thing that would typically be caught by a linter if the project cares about it, 4) Not clearly wrong - some people intentionally use four periods for emphasis. The rules state not to make comments that are "obvious or unimportant" and this falls into the unimportant category. It's also purely stylistic and doesn't affect code behavior. Could this be part of a broader style guide that the project follows? Maybe the author intentionally used four periods for emphasis or to match other similar messages in the codebase. Without seeing the full context of other messages, I can't be certain this is actually inconsistent. Even if there's a style guide, this is such a minor punctuation detail that it's unlikely to be worth a PR comment. The automated tool should not be nitpicking about whether an ellipsis has 3 or 4 periods unless there's clear evidence this violates an important project standard. This falls under "obvious or unimportant" comments that should be removed. This comment should be deleted. It's an extremely minor stylistic nitpick about punctuation that doesn't affect functionality and falls under the category of unimportant comments that shouldn't be made.

Workflow ID: wflow_NWoeHS3iOBzw7KDx

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@mrhoribu
Copy link
Contributor

mrhoribu commented Dec 5, 2025

@ellipsis review again

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 b0a6d4e in 26 minutes and 28 seconds. Click for details.
  • Reviewed 3182 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 14 draft 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/bigshot.lic:416
  • Draft comment:
    In the buff‐check block of command_check, you use the entire command string (via 'command') to look up the buff name instead of the current modifier token. This seems unintended – consider using buff_match[1].downcase to index @COMMAND_BUFF_CHECKS.
  • 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.
2. scripts/bigshot.lic:705
  • Draft comment:
    The new boon‐support functions check the creature’s type using ‘creature.type.include?("boon")’ which is case–sensitive. For robustness, convert the type to lowercase (e.g. creature.type.to_s.downcase.include?("boon")) so that variations in capitalization are handled.
  • 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.
3. scripts/bigshot.lic:628
  • Draft comment:
    The method leader_target? (a predicate method) returns GameObj.target rather than a boolean value. Consider renaming it (e.g. to leader_target) or explicitly returning a boolean (e.g. !!GameObj.target) for clarity.
  • 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.
4. scripts/bigshot.lic:1026
  • Draft comment:
    Typo detected: The key 'group_deader' appears to be a misspelling. Perhaps you meant 'group_leader'?
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
5. scripts/bigshot.lic:1124
  • Draft comment:
    Consider revising the text in the Notes section. The sentence starting with "Bigshot is designed to handle combat routines..." runs into the next sentence beginning with "Afk scripting..." without proper punctuation. It might be clearer to end the first sentence with a period and consider capitalizing "Afk" to "AFK" if that is the common usage.
  • Reason this comment was not posted:
    Comment was on unchanged code.
6. scripts/bigshot.lic:1193
  • Draft comment:
    Typo detected: In the label text 'Note: Use nouns or name for flee info (case sensative).', 'sensative' should be corrected to 'sensitive'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
7. scripts/bigshot.lic:1403
  • Draft comment:
    Typo detected: The widget id 'group_deader' appears to be misspelled. It likely should be 'group_leader' (or another intended variant). Please correct this.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The comment assumes group_deader is a typo for group_leader, but based on the context, group_deader appears to be intentional. The feature is about stopping when group members are dead (hence "deader" as in "dead-er" or someone who is dead). The label "Stop for dead group members" confirms this is about detecting dead players, not about the group leader. The variable name follows the pattern of other boolean flags in the codebase. This comment appears to be incorrect. Could "deader" be slang or informal terminology that I'm not familiar with? Maybe in the gaming community "deader" is a common term? However, even if it's informal, the functionality clearly matches the name - it's about dead group members, not the leader. Even if "deader" is informal or slang, the functionality is clear and consistent. The label explicitly says "Stop for dead group members" and the code checks for dead status. The comment is making an incorrect assumption about what the intended name should be. This comment should be deleted. The widget id group_deader is not a typo - it's an intentional name for a feature that stops hunting when group members are dead. The functionality, label, and usage throughout the code all confirm this is the correct name.
8. scripts/bigshot.lic:1988
  • Draft comment:
    Typo in comment: 'turn everyting off' should be 'turn everything off'.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% This is a straightforward typo fix in a code comment. The rules state "Do NOT comment unless there is clearly a code change required" and "Do NOT make comments that are obvious or unimportant." A typo in a comment doesn't affect functionality. However, it does improve code quality and readability. The comment is actionable and clear - it's a simple one-character fix. But is fixing a typo in a comment "clearly a code change required"? It's borderline. The rules emphasize not making obvious or unimportant comments, and a single-letter typo could be considered unimportant. While typos should generally be fixed, this is a very minor issue in a comment (not code). The rules explicitly say not to make "obvious or unimportant" comments. A single-letter typo in a comment that doesn't affect understanding could be considered unimportant. The PR author might find this nitpicky. However, the comment is correct, actionable, and improves code quality with minimal effort. It's not speculative or asking for confirmation - it's a clear fix. Some teams value clean comments and documentation. The suggestion format makes it easy to apply. This is a borderline case. The typo fix is valid but very minor. Given the rules emphasize not making unimportant comments, and this is just a single-letter typo in a comment (not affecting functionality), I should lean toward deleting it.
9. scripts/bigshot.lic:2830
  • Draft comment:
    Typographical note: The key 'group_deader' might be a typo. Possibly it was intended to be 'group_dead' (or similar) to reflect the comment 'stop for dead followers'. Please verify the intended naming.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
10. scripts/bigshot.lic:4951
  • Draft comment:
    Typo: In the comment "TODO PoPped muscle check updates", 'PoPped' seems to have an inconsistent capitalization. Consider changing it to 'popped' for clarity.
  • 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.
11. scripts/bigshot.lic:5652
  • Draft comment:
    Typographical Issue: The debug message text uses 'croak_script' (singular) even though the method is named 'croak_scripts' (plural). Please update the debug message for consistency.
  • Reason this comment was not posted:
    Comment was on unchanged code.
12. scripts/bigshot.lic:5793
  • Draft comment:
    Typo: In the comment "tails wont have followers", consider changing "wont" to "won't" for grammatical correctness.
  • 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.
13. scripts/bigshot.lic:6118
  • Draft comment:
    Typo: The message text currently uses four consecutive periods (....). Typically an ellipsis is three dots (...). Consider reducing the period count if this was not intentional.
  • Reason this comment was not posted:
    Comment was on unchanged code.
14. scripts/bigshot.lic:8049
  • Draft comment:
    Typographical error: In the comment, "even" should be replaced with "event".
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a comment about a typo in a code comment (not code itself). According to the rules, I should not comment on things that are "obvious or unimportant." A typo in a comment is relatively minor and could be considered unimportant. However, the rules also say comments suggesting code quality refactors are good if they are actionable and clear. This is actionable (fix the typo) and clear. But is fixing a typo in a comment really a "code quality refactor"? It's more of a documentation/clarity improvement. The rules emphasize not making obvious or unimportant comments. A single-letter typo in a comment seems like it falls into the "unimportant" category - it doesn't affect code behavior, readability is barely impacted, and it's extremely minor. The typo fix is technically correct and would improve the comment slightly. It is actionable and clear. However, this might be too minor to warrant a PR comment - it's just one letter in a comment that doesn't affect functionality. The rules say not to make "obvious or unimportant" comments, and this could fall into the "unimportant" category. While the typo is minor, fixing typos does improve code quality and professionalism. However, the rules specifically say not to make obvious or unimportant comments. A single-letter typo in a comment is quite unimportant in the grand scheme of code review. The developer can catch this themselves or it can be fixed in a future cleanup. This doesn't rise to the level of importance that warrants a PR comment. This comment should be deleted. While technically correct, it points out an extremely minor typo in a code comment that has no functional impact. This falls under the "unimportant" category of comments that should not be made according to the review rules.

Workflow ID: wflow_rHvCfZ2kIsWpY9nd

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants