Skip to content

Conversation

@MahtraDR
Copy link
Collaborator

@MahtraDR MahtraDR commented Feb 8, 2026

Summary

Comprehensive refactoring of forge.lic following the established dr-scripts/lich-5 refactoring patterns. This PR adds new features, fixes bugs, improves code maintainability, and adds comprehensive test coverage.

New Features

  • finish argument - Optional argument to specify what happens to completed items:
    • hold (default) - Leave item in hand
    • log - Log to engineering logbook
    • stow - Stow in crafting container
    • trash - Dispose of item
  • Automatic forge rental renewal - Proactively checks rental status at startup and renews if <10 minutes remaining. Also monitors for low rental warning during crafting and auto-renews.

Bug Fixes

  • @debug assignment order - Fixed bug where @debug was used in echo statement before being assigned (line 69 in original)

Code Quality Improvements

  • frozen_string_literal: true pragma added
  • 30+ pattern constants extracted - All inline pattern arrays/strings moved to frozen class constants for clarity and reuse
  • Regexp.last_match → named captures - Rental time parsing now uses explicit named capture (expire_time) instead of global state
  • Lich::Messaging.msg with Forge: prefix - All echo/DRC.message calls converted to proper messaging API with consistent module prefix
  • DRCI predicate conversions - get my ingotDRCI.get_item? with proper failure handling and messaging
  • Verbose failure messaging - Every exit/failure path now logs a descriptive error message explaining what went wrong
  • Refactored work() method - Large case statement split into focused handler methods (handle_fuel_needed, handle_pounding, handle_grindstone, handle_pliers, handle_oiling, handle_handle_assembly, handle_roundtime) for maintainability

Constants Added (30+)

Constant Description
RENTAL_EXPIRE_PATTERN Named capture regex for rental expiration
RENTAL_NOT_FOUND_PATTERNS Notice not found responses
RENTAL_RENEW_SUCCESS/FAILURE_PATTERNS Rental renewal responses
TONGS_ERROR_PATTERNS Tongs-related error messages
FUEL_NEEDED_PATTERNS Forge fuel low messages
BELLOWS_NEEDED_PATTERNS Bellows needed messages
TONGS_TURN_PATTERNS Turn with tongs messages (6 patterns)
COOLING_PATTERNS Slack tub/cooling messages
POUND_PATTERNS Pounding needed messages (6 patterns)
GRINDSTONE_PATTERNS Grindstone work messages (6 patterns)
PLIERS_PATTERNS Pliers work messages (5 patterns)
OIL_PATTERNS Oiling needed messages (4 patterns)
SPIN_SUCCESS/FAILURE_PATTERNS Grindstone spinning results
ASSEMBLE_SUCCESS_PATTERNS Assembly success messages
STAMP_PATTERNS Stamping result messages
+ 15 more single-value constants

New Files

  • spec/forge_spec.rb - 83 comprehensive tests covering:
    • All 30+ constants (frozen, correct values)
    • resolve_recipe_name conversion logic
    • debug_log / error_log messaging
    • check_rental_status parsing and renewal logic
    • renew_forge_rental success/failure paths
    • find_item location detection
    • restow_ingot with failure handling
    • All handler methods (handle_fuel_needed, handle_pounding, etc.)
    • assemble_part with missing part handling
    • spin_grindstone success/failure/missing paths
    • swap_tool with adjustable tongs logic
    • check_hand swap/missing scenarios
    • finish with all finish options (hold/log/stow/trash)
    • set_defaults for all recipe types

Test plan

  • Created 83 RSpec tests covering all functionality
  • All tests passing locally
  • Rubocop clean (no offenses)
  • Manual in-game testing of:
    • Normal forging workflow
    • finish log option
    • finish stow option
    • finish trash option
    • Rental renewal at <10 minutes
    • Rental warning at 10-20 minutes
    • Resume functionality

Migration Notes

  • No breaking changes - all existing usage patterns continue to work
  • New finish argument is optional, defaults to hold (existing behavior)
  • Rental check is automatic and transparent

🤖 Generated with Claude Code


Important

Refactors forge.lic to add finish option, automatic rental renewal, and robust patterns, with comprehensive tests in spec/forge_spec.rb.

  • Behavior:
    • Adds finish argument to Forge class in forge.lic with options hold, log, stow, trash.
    • Implements automatic forge rental renewal in check_rental_status and renew_forge_rental.
  • Code Quality:
    • Extracts 30+ pattern constants in forge.lic for clarity and reuse.
    • Refactors work() method into smaller handlers like handle_fuel_needed, handle_pounding.
    • Replaces Regexp.last_match with named captures.
  • Testing:
    • Adds spec/forge_spec.rb with 83 tests covering constants, methods, and new features.
  • Misc:
    • Adds frozen_string_literal: true pragma to forge.lic.
    • Converts echo calls to Lich::Messaging.msg with Forge: prefix.

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

…robust patterns

## Summary
- Add optional `finish` argument (hold/log/stow/trash) for completed items
- Add automatic forge rental renewal system with proactive checking
- Extract 30+ inline patterns to frozen class constants
- Replace all echo/DRC.message with Lich::Messaging.msg with Forge: prefix
- Replace Regexp.last_match with named capture for rental time parsing
- Add DRCI predicate conversions for get/put operations with failure messaging
- Add verbose error messaging on all exit/failure paths
- Refactor work() method into smaller handler methods for maintainability
- Fix @debug assignment order bug (was used before assigned)

## Test plan
- 83 comprehensive RSpec tests covering all constants, methods, and edge cases

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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 b4e63c9 in 17 seconds. Click for details.
  • Reviewed 1672 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 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.

Workflow ID: wflow_6GqbL0rqN6HfzuPo

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

MahtraDR and others added 6 commits February 9, 2026 12:49
The CI environment loads the real Flags module from lich-5 while the
test harness defines Flags as a class. This adds singleton methods to
Flags to ensure it has []=, [], reset, add, and delete methods regardless
of whether Flags is a module or class.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The previous fix only checked for []= and skipped all methods if that
existed. CI may have a partial Flags implementation with some methods
but not others. Now each method is checked individually before defining.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Define Flags as a class at the top of the spec file BEFORE loading the
test harness. This prevents conflicts with other spec files that may
define Flags differently (e.g., workorders_spec.rb defines it as a
module with fewer methods).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace global Flags class definition with stub_flags_class helper
that uses RSpec's stub_const. This avoids conflicts with other specs
that define Flags differently (workorders_spec.rb defines it as module).

Call stub_flags_class in each describe block that needs Flags:
- check_rental_status
- renew_forge_rental
- restow_ingot
- handle_roundtime
- assemble_part

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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.

1 participant