feat(map.lic): v2.0.0 refactor

[mrhoribu]

Important

Refactor map.lic into ElanthiaMap namespace with improved architecture, features, and settings management.

• Namespace and Architecture:
  • Refactor into ElanthiaMap namespace with module/class architecture.
  • Introduce MapData, MapCache, and Window classes for better organization.
• Features and Functionality:
  • Add "Keep Centered" option to auto-center viewport on room marker.
  • Implement comprehensive YARD documentation.
  • Modernize Ruby idioms and clean up GTK3 code.
  • Add support for dark mode and dynamic menu rebuilding.
• Settings and UI:
  • Enhance settings management with load_settings and save_settings methods.
  • Improve UI with context menus for map, tags, locations, and settings.
  • Implement drag-and-drop and zoom functionality with mouse events.
• Miscellaneous:
  • Rename legacy 'narost' variables to descriptive names.
  • Add error handling and debug output for troubleshooting.

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

[mrhoribu]

@ellipsis remove previous comments and review again

[mrhoribu]

@ellipsis remove previous comments and review again

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to b04bc78 in 2 minutes and 11 seconds. Click for details.

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

• Draft comment:
  The thread in start_drag polls pointer positions using sleep in a loop. Consider reducing poll frequency or using an event-driven approach to improve responsiveness and avoid busy waiting.
• 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 comment is suggesting a refactor to improve responsiveness and avoid "busy waiting". However, the code is NOT busy waiting - it's sleeping between polls (0.05s + 0.01s waits). The polling frequency seems reasonable for a drag operation. The suggestion to use "event-driven approach" is vague - GTK drag operations often DO require polling the pointer position in a loop like this. The comment doesn't explain what specific event-driven approach would work better, and doesn't demonstrate that there's actually a problem with the current implementation. This feels like a speculative code quality suggestion rather than identifying a clear bug or issue. The rules say "Comments that suggest code quality refactors are good! But only if they are actionable and clear." This comment is not actionable (no clear alternative provided) and doesn't demonstrate a real problem. Maybe there IS a better GTK-native way to handle drag operations that I'm not aware of? Perhaps the polling approach is genuinely suboptimal for GTK applications? The comment might be pointing to a real performance issue even if not clearly explained. Even if there's a theoretically better approach, the comment doesn't provide enough detail to be actionable. It doesn't explain what "event-driven approach" means in this context, doesn't cite GTK best practices, and doesn't demonstrate that the current 50ms polling is causing any actual problems. The code is already using Gtk.queue for thread safety. Without more specific guidance, this is too vague to be useful. This comment should be deleted. It's a vague suggestion for improvement without demonstrating a real problem or providing actionable guidance. The code uses reasonable sleep intervals (not busy waiting), and the comment doesn't explain what specific event-driven alternative would work better in GTK.

2. scripts/map.lic:217

• Draft comment:
  The regex in normalize_key may not handle filenames with multiple dots or unusual characters. Consider more robust normalization.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

3. scripts/map.lic:504

• Draft comment:
  capture_window_geometry waits in a sleep loop until a flag is set. Consider using a callback mechanism or event-driven approach to avoid potential delays or hangs.
• 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 common pattern when working with GTK from Ruby - you need to queue operations on the GTK thread and wait for them to complete. The alternative (callbacks) would be more complex and harder to reason about for this use case. The sleep loop with timeout is appropriate here because: 1) It's only used during cleanup, not hot path, 2) The timeout prevents hangs, 3) GTK.queue doesn't provide a built-in synchronous wait mechanism, 4) The sleep duration (0.01s) is reasonable. The comment seems to be suggesting a refactor based on general principles rather than identifying an actual problem with this code. Maybe there's a GTK-specific synchronization primitive I'm not aware of that would be cleaner? Or perhaps the comment author has experienced actual hangs with this pattern in practice? Even if there were a cleaner GTK synchronization mechanism, this code works correctly and is only called during cleanup. The timeout prevents hangs. Without evidence of actual problems, this is just a style preference. The comment doesn't point to a bug or clear improvement. This comment suggests a refactor without identifying an actual problem. The sleep loop with timeout is an appropriate pattern for synchronizing with GTK's event loop during cleanup operations. Delete the comment.

4. scripts/map.lic:1309

• Draft comment:
  The if/elsif chain in handle_click for modifier keys is long. Consider refactoring for clarity, possibly abstracting modifier-handling logic into helper methods.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

5. scripts/map.lic:338

• Draft comment:
  resolve_map_path hard-codes replacement of '/maps/' with '/maps-dark/'. Ensure this assumption about the folder structure holds in all deployments.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the author to ensure that a specific assumption about folder structure holds in all deployments. This is a request for confirmation about the intention and deployment environment, which violates the rules.

6. scripts/map.lic:2005

• Draft comment:
  The main loop uses $room_count and sleep polling for room changes. Document the purpose of these global variables and consider event-driven updates if available.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

7. scripts/map.lic:1674

• Draft comment:
  prompt_find_room rescues exceptions when searching for a room but only logs a message via respond. Consider more detailed logging or user feedback if the lookup fails.
• Reason this comment was not posted:
  Confidence changes required: 40% <= threshold 50% None

8. scripts/map.lic:2040

• Draft comment:
  Throughout the script, the 'respond' function is used for debug output. Verify that no sensitive information is output in production, especially in trouble mode.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

9. scripts/map.lic:1977

• Draft comment:
  Overall, the refactoring into a namespace and class-based architecture is a solid improvement for maintainability. Consider adding some unit tests for interactive functions if possible.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

10. scripts/map.lic:31

• Draft comment:
  Typo: Consider using "dropdown" (one word) instead of "drop down" 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.

Workflow ID: wflow_Gx2Z6ttlFqTMc7EB

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