fix(familiar.lic) - updates to check if 920 is known and script syntax modernization

[AndreasWelch]

Important

Updates familiar.lic to check if spell 920 is known before casting, modernizes script syntax, and updates documentation.

• Behavior:
  • Added check in familiar.lic to ensure spell 920 is known before casting, preventing infinite error loop.
  • Removed hide_me as it was unnecessary.
• Syntax Modernization:
  • Updated script syntax for better readability and adherence to current standards.
  • Replaced echo with Lich::Messaging.msg for consistent messaging.
• Documentation:
  • Updated version to 1.46.0 and changelog to reflect changes.
  • Fixed header format and version formatting in familiar.lic.

^{This description was created by for 373ddac. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 24b54a5 in 2 minutes and 1 seconds. Click for details.

• Reviewed 163 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 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/familiar.lic:68

• Draft comment:
  Redundant spell check: Spell[920].known? is verified both in the help block (line 68) and immediately after (line 72). Consider consolidating these checks.
• 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 two checks are NOT redundant. Line 68 is inside the help block (lines 55-70) which only runs when the user passes "help" as an argument, and it exits immediately after at line 69. Line 72 is the main validation check that runs when the script starts normally (without the help argument). The help block check is just informational to tell the user they don't know the spell when viewing help. The line 72 check is the actual guard that prevents the script from running. This is a valid pattern - showing the error in help output AND preventing execution. The comment suggests consolidation but doesn't understand the control flow. Could the author have intended for these to be consolidated for cleaner code? Maybe there's a way to structure this that avoids the duplication while maintaining the same functionality. Perhaps I'm being too generous in assuming the current structure is intentional. While consolidation might be possible, the current structure is functionally correct and not actually redundant due to the control flow. The help block exits before reaching line 72, and the line 72 check only runs during normal execution. This is a common pattern and the comment misunderstands the code flow. The comment is not identifying a bug or clear improvement. The comment incorrectly identifies these checks as redundant. They serve different purposes in different execution paths (help vs normal execution) and never both execute in the same run. The comment should be deleted as it misunderstands the control flow.

2. scripts/familiar.lic:56

• Draft comment:
  Repeated messaging calls in the help block: The series of Lich::Messaging.msg calls (lines 56–67) could be refactored into a helper method for clarity and easier maintenance.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

3. scripts/familiar.lic:41

• Draft comment:
  Typographical error: In the changelog entry, "fizzle/interfence" should be corrected to "fizzle/interference".
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_xw3s8Vw2b25VXw5Y

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

[Copilot]

Pull request overview

This PR modernizes the familiar.lic script by adding a check to verify spell 920 (Call Familiar) is known before execution, preventing infinite error loops. The update also migrates from deprecated echo statements to Lich::Messaging.msg calls throughout the script and updates header formatting to current conventions.

Key Changes:

• Added spell 920 knowledge validation before script execution with user-friendly error messages
• Converted all echo statements to Lich::Messaging.msg for consistency with modern Lich scripting standards
• Enhanced help text to display current configuration values dynamically

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 373ddac in 1 minute and 43 seconds. Click for details.

• Reviewed 165 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 8 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/familiar.lic:10

• Draft comment:
  Documentation now shows --allowmove=<YES/NO>, matching underlying boolean calls. Confirm that user input (yes/no) is well‐handled everywhere and documented (e.g. check index usage in parameter parsing).
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. scripts/familiar.lic:47

• Draft comment:
  Removal of the 'hide_me' command is intentional. Verify that exposing the script is acceptable for your environment.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

3. scripts/familiar.lic:69

• Draft comment:
  The check for Spell[920].known? appears twice (in the help branch and immediately after). Consider consolidating to avoid duplicate logic.
• 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% The two checks actually serve different purposes. The first check (line 69) is inside the help display and only shows a warning message if the user doesn't know the spell while viewing help. The second check (line 73) is a guard clause that prevents the script from running at all if the user doesn't know the spell. The help branch exits regardless of whether the spell is known, so the second check is necessary for normal execution. While there is technically duplicate logic, consolidating them would make the code less clear and potentially break the help functionality. This appears to be a case where the duplication is intentional and appropriate. Could the author have intended to consolidate this for DRY principles? Perhaps extracting this into a method would be cleaner. The comment is about code quality, which the rules say is good if actionable and clear. While DRY is generally good, these checks serve different contexts (help vs. execution) and consolidating them would likely make the code more complex rather than simpler. The suggestion isn't actionable in a clear way - it doesn't specify how to consolidate them without breaking the logic. The current structure is clear and intentional. This comment should be deleted. The duplicate check serves different purposes in different contexts (informational warning in help vs. guard clause for execution), and consolidating them would not improve the code. The suggestion is not clearly actionable.

4. scripts/familiar.lic:56

• Draft comment:
  Help text is triggered by checking variable[1] while option parsing later uses variable[0]. Ensure the ordering and documentation for command-line arguments is unambiguous.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

5. scripts/familiar.lic:53

• Draft comment:
  Default refresh_timer is set as a float (15.0) while later parsed as an integer. Consider using a consistent data type for clarity.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

6. scripts/familiar.lic:97

• Draft comment:
  In the branch handling fizzle/interference, after moving to a node a second Spell[920].cast is executed. It might be beneficial to check this cast’s success before returning to the original room.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

7. scripts/familiar.lic:66

• Draft comment:
  Typographical suggestion: In the help message, the phrase "sets time to refresh 920 too in minutes" seems a bit off. Consider removing or replacing "too" (e.g., "sets time to refresh 920 in minutes" or "sets time to also refresh 920 in minutes") to improve clarity.
• Reason this comment was not posted:
  Comment was on unchanged code.

8. scripts/familiar.lic:113

• Draft comment:
  Typo: The string "re-fresh" may be intended to be "refresh" (without a hyphen) for consistency. Please confirm if a hyphen is intended.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_bCGROEWPWBa3aPNS

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