-
Notifications
You must be signed in to change notification settings - Fork 41
Minor fix for fuzzy exit name match #402
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
Minor fix for fuzzy exit name match #402
Conversation
Changes: - Adds priority checking for standard direction abbreviations in FindExitByName - Implements direct mapping for compass directions before fuzzy matching - Supports case-insensitive abbreviation matching - Preserves fuzzy matching for custom exit names (portal, gate, etc.) This ensures deterministic behavior where "n" always maps to "north" when that exit exists, eliminating problems in exit name resolution during rapid movement.
|
I think in theory, any translation from alias to full exit name should happen before here, although there may be a reason why we can't do that in some cases. We do define directional aliases here though: https://github.com/GoMudEngine/GoMud/blob/master/_datafiles/world/default/keywords.yaml#L233-L243 Maybe there's somewhere up-chain that makes more sense? Do we understand why rapid movement causes this to occur? |
|
I may have been incorrect in stating that it's when rapid movement occurs. Upon further testing, I can confirm that it's also when moving slowly. The real issue, which I have not communicated clearly, is that exits like "northeast, northwest, southeast and southwest" enforce an exact match. When we then have an exitNames variable that is built randomly every time from the exit keys, sometimes "north" comes first and sometimes "northeast" comes first. When the fuzzy match matches "n" to "north" everything is fine, but when it matches "n" to "northeast" there is an extra condition on "northeast" that enforces an exact match. When this then fails, the entire function returns false. When I use "north" as the exit command, it finds an exact match to the north exit, and does not have to use the fuzzy match, and never goes into the code block that enforces an exact match to northeast. Let me try and rewrite this to leverage the directional aliases first, and then fuzzy match. This should enable us to completely remove the enforced exact match to "northeast" etc. As you know, I am not a professional developer, and I'm on a steep learning curve with this. I really appreciate your patience and willingness to give guidance and suggestions for how to do something in a different way, given your knowledge of the codebase. Thanks :) |
- Remove hardcoded standardDirectionAbbrevs map from rooms.go - Remove exact match enforcement for compound directions (northeast, etc.) - Use keywords.TryDirectionAlias() to resolve direction shortcuts from keywords.yaml - Simplify fuzzy matching logic to work uniformly for all exits This allows direction aliases like "n" for "north" to be configured in keywords.yaml rather than hardcoded, and removes the restriction that prevented fuzzy matching on compound directions.
|
Ok, refactored to use the directional aliases. This make a lot more sense. Thanks again for the suggestion. |
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.
Pull Request Overview
This PR fixes the exit matching logic in rooms by prioritizing standard direction abbreviations through a direct keyword lookup before falling back to fuzzy matching.
- Adds a direct mapping for compass directions using keywords.TryDirectionAlias.
- Implements case-insensitive matching and retains fuzzy matching for custom exit names.
Comments suppressed due to low confidence (1)
internal/rooms/rooms.go:1765
- [nitpick] Ensure that removing the previous check for specific exact matches (e.g., for 'southeast', 'southwest', etc.) does not reintroduce ambiguous matching behavior for standard cardinal directions; if this is intended behavior, adding a clarifying comment would improve maintainability.
if len(exactMatch) == 0 {
| func (r *Room) FindExitByName(exitNameSearch string) (exitName string, exitRoomId int) { | ||
|
|
||
| // Check for direction aliases from keywords.yaml first | ||
| fullDirection := keywords.TryDirectionAlias(exitNameSearch) |
Copilot
AI
Jun 19, 2025
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.
[nitpick] Consider adding a comment or refactoring to clarify that fuzzy matching is purposefully limited to r.Exits (excluding temporary and mutator exits) to ensure consistent behavior for direction abbreviations.
Fixed an issue where abbreviated direction commands (n, s, e, w, etc.) would fail with "command not recognized" when moving rapidly between rooms. The bug specifically occurred in rooms with ambiguous abbreviations (e.g., rooms with both "north" and "northeast" exits).
Changes:
This ensures deterministic behavior where "n" always maps to "north" when that exit exists, eliminating problems in exit name resolution during rapid movement.