fix(trades): reject rate terms that cannot describe a trade - #40
Merged
Conversation
`parseRate` only screened its result for `NaN`, so three kinds of nonsense came back as numbers and travelled on into `sellMetal` and friends as if they were answers: '0:1.5:1' -> another term divided by zero, so Infinity resources '2::1' -> an empty term parses to 0, so 0 of that resource, silently '-2:1.5:1' -> negative resources The check now runs on the normalized terms and demands a real, strictly positive number, which is the only shape a worth can take. It stays on the result rather than the raw input because every one of those cases surfaces there as a NaN, an Infinity, a 0 or a negative ratio — and because the unknown-reference-resource error must still come first. Callers that passed a usable rate see no change. `ogame-ui` already screened its custom-rate form for exactly these three, since the library would not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 4.0.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This was referenced Aug 15, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #39, which noted this in passing.
The bug
parseRatescreened its result withhasNaN, which only catchesNaN. Three other kinds of nonsense came back as numbers and travelled on intosellMetal/sellCrystal/sellDeutas if they were answers:'0:1.5:1'{ deut: Infinity, crystal: Infinity }'2::1'{ deut: 25, crystal: 0 }0, silently'-2:1.5:1'{ metal: -100, crystal: 75 }None of the three describes a trade.
Infinityand0are the bad ones: they reach a UI and get rendered.The fix
The check now demands a real, strictly positive number for every normalized term — the only shape a worth can take.
src/trades/utils.jsgoes fromhasNaNtoisUsableRate, its only caller being this one.It stays on the result rather than the raw input for two reasons: every one of those cases surfaces there (as a
NaN, anInfinity, a0or a negative ratio), and the unknown-reference-resource error must keep firing first —parseRate('0:0:0', 'gold')still saysgold is not part of the game, which a test now pins.Compatibility
No change for any usable rate. What changes is that three inputs which used to return a number now throw, which is what the README already promised ("Throws if the rate is malformed") and what the only shipped consumer already assumed:
ogame-uiscreens its custom-rate form before calling, precisely because the library would not — so it never reaches this path.og-bot-discordwraps every command in a try/catch, so a malformed rate falls back to the help message instead of printingInfinity. It is being bumped to v4 separately and gets its own message for this.An all-negative rate like
'-2:-1.5:-1'still passes, because it normalizes to exactly the same ratios as'2:1.5:1'— it is the same rate, written oddly.Tests
0, empty and negative terms rejected across the three reference resources, the unknown-resource error still named, plusisUsableRatecovered directly. Lint, types and 325 tests pass locally.🤖 Generated with Claude Code