Skip to content
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

Attack mixup if weapon name differs only in a postfix #2025

Closed
StefanLeng opened this issue Nov 9, 2024 · 2 comments · Fixed by #2026 or #2038
Closed

Attack mixup if weapon name differs only in a postfix #2025

StefanLeng opened this issue Nov 9, 2024 · 2 comments · Fixed by #2026 or #2038

Comments

@StefanLeng
Copy link
Contributor

StefanLeng commented Nov 9, 2024

I stumbeld on an issue during test for my own code, I am not sure if that is a realistic scenario in the wild:

When importing a character that has two simmilar weapons where the name of one is the name of another + a prefix all lookks good after the initial import:

Weapon_mixup1

But if I import the same character again, the attacks get mixed up:

Weapon_mixup2

I tracked tthe issue to this pice of code:

https://github.com/crnormand/gurps/blob/09dcc1d74fe16d3b8e45ee8f408883fa1f4a366d/module/actor/actor-importer.js#L2322C1-L2323C74

From _findElementIn the last attack where the start of the name matches is returned, which leds to one set of attacks beeing update from worng matches.

I am not sure why we can't use an exact match here, as we are matching an attack to an attack (I am aware we need to use startsWith when matching a attack to the weapon in the equipment, but that is another case), but even if we need that, we should use the best match, not the last.

Test character in atttached zip. test.zip

If you can give advise what the actual matching rule here should be, I am happy to prepare a fix, if you want.

@mjeffw
Copy link
Collaborator

mjeffw commented Nov 9, 2024

I think @NoSe was always for being lenient in checks in case the user adds text to the imported name. I think the best fix is to get all matches and update the best match.

Alternatively, I'm okay with exact match as well.

@StefanLeng
Copy link
Contributor Author

Thats a good point! I go for the best match (that is, the shortest entry that matches).

@mjeffw mjeffw linked a pull request Nov 27, 2024 that will close this issue
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 a pull request may close this issue.

2 participants