feat(save-editor): edit what a merchant sells and how much ore he has - #84
feat(save-editor): edit what a merchant sells and how much ore he has#84dh0er wants to merge 6 commits into
Conversation
A merchant's shop is not his inventory. It lives in one global array (m_Traders) keyed by his unique name, and holds two maps: the live stock and the baseline he restocks toward. His ore sits in that same map as an ordinary line, because ore is the currency and what he holds is what he can pay with. Adds private.traders.list/.detail plus three edits: setStock (a bare i32 at the tail of its map entry, so it batches), and addItem/removeItem (structural splices, so they stand alone in their write and are listed in both the core guard and the app's splicingPaths). Two things the data forces: - Rows are addressed by array index, never by name. Two shipped rows are named `None`, are byte-identically long, and hold the same ore, so a name lookup has to refuse rather than guess. - A sold-out item is deleted from the map, not left at zero. setStock therefore refuses a line that does not exist and points at addItem instead of reporting success for a write it cannot do. Ore is optional: three merchants carry no ore line at all, which reads as null rather than zero so the UI does not claim they are broke. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… browsed Queued changes now sit above the list rather than inside it. An addition has no row yet and a removal's row is about to stop existing, so drawing either among the saved lines claimed a state the save does not have. Both use the same banner the inventory uses, extracted into pending_structural_row.dart. The two stock maps get a segmented switch instead of sitting on one page. That also settles which map the ore field belongs to: the ore card only appears for the live stock, where the number IS the merchant's purchasing power, and its line leaves the list so it is not shown twice. In the restock baseline ore is an ordinary row, because "purchasing power" means nothing there. Stock is grouped by item category behind the inventory's own sidebar, sorted by the localized name with the class id as tiebreak. Smaller corrections from testing against a real save: - The ore hint said what ore is for. It now says what the user cannot see otherwise: the in-game figure differs, because on load the game adds what accrued since the merchant's last trade. - Class ids followed the show-object-ids setting nowhere; now they do, and are dropped entirely when the title already is the id. - The character list badges a merchant, joined in the core so the list needs no second query. - The price note moved to the top, where it qualifies the ore as much as the counts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ac681af. Configure here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac681afa10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two reports from the PR review. A count of 0 was accepted and written in place. Sold-out lines are deleted from the map rather than held at zero — no shipped or played save carries a zero-valued entry — so that write invented a record the game never produces. setStock now requires a positive count and names removeItem in the refusal, which is what "he no longer offers this" actually looks like. addItem already rejected zero for the same reason. The count field stops submitting it. The count field also kept the previous line's value. It refreshed only when the queued count changed, and rows carried no key, so the list reused one line's field state for whichever line landed at that position after a category or map switch — and submitting the leftover queued a setStock against the item now shown. Rows are keyed by item path, and the field now reacts to a changed saved value as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dca43e6870
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…line removable Three reports from the second review pass. The core badges a merchant by matching lowercased names, while a character's unique name is the stored knowledge key where one exists — whose casing can differ from the trader row's. The app compared exactly, so a character the list badged as a merchant could open on "does not trade". It now folds case the same way the core does. Pulling the ore row out of the live-stock list left no way to drop it, and setStock no longer accepts zero — yet a merchant with no ore line at all is a state the game itself produces. The ore card carries a delete action now, and its field is disabled while that removal is queued. The count field accepted values above i32::MAX, which the core then refused at save time. It applies the same bound the add-item dialog already applies, so the refusal happens in the field instead of at the end of a save. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2897b09d21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
apply_set_stock already refuses it; apply_add_item only refused a negative, leaving a Rust caller able to insert the zero-valued entry the module treats as an invalid state. The JSON parser was the only thing enforcing a positive count on that path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12a1091446
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…led stock private.traders.detail compared trader names exactly, so a caller passing the uniqueName that private.characters.list returned — the stored knowledge key, whose casing can differ from the trader row's — got "no trader named" for a character the same list had marked a merchant. The Dart-side fold did not cover direct users of the command. The ambiguity check still runs, now on the folded comparison, so two rows differing only in case are still refused. A save whose per-difficulty stock is populated now disables editing and says why. The edits reach only m_Items and m_DefaultItems, so such a save would have taken a change, reported success, and left that stock standing. Empty in every save observed so far, which is exactly why it needed saying. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8274c5b8be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (final item in pendingAdds) ...[ | ||
| const SizedBox(height: 8), | ||
| _PendingLineRow( | ||
| item: item, | ||
| tone: PendingTone.add, | ||
| onCancel: () => onRevertAdd(map, item.path), | ||
| ), |
There was a problem hiding this comment.
When a user queues enough additions or removals—for example while replacing most of a merchant's stock—every pending banner is inserted as a fixed child above the only Expanded list. _StockSection itself has bounded height and provides no scrolling around these banners, so their combined height eventually causes a vertical RenderFlex overflow, hides the stock browser, and can make later pending changes impossible to reach or cancel. Put the pending rows in a bounded scrollable area or include them in the main list.
Useful? React with 👍 / 👎.
1 participant
A merchant's shop is not his inventory. It lives in one global array
(
m_Traders) keyed by his unique name and holds two maps: the live stock andthe baseline he restocks toward. His ore sits in that same map as an ordinary
line, because ore is the colony's currency and what he holds is what he can pay
with.
Adds a Handel sub-tab between Inventar and Dialogwissen, and the core
commands behind it.
Core
private.traders.list/.detail, plus three edits:setStock— a bare i32 at the tail of its map entry, so it batches with peers.addItem/removeItem— structural splices, classified inmay_invalidate_caller_ordinalsandcarries_caller_ordinalso the core'sordering rule keeps them apart from index-addressed peers. The app mirrors
that classification, which is what decides how a save is split into writes.
Two things the data forces:
named
None, are byte-identically long and hold the same ore, so a namelookup refuses rather than guesses.
setStocktherefore refuses a line that does not exist and points at
addIteminsteadof reporting success for a write it cannot do.
Ore is optional: three merchants carry no ore line at all, which reads as
nullrather than0so the UI does not claim they are broke.private.characters.listgainsisTrader, joined in the core so the characterlist can badge merchants without a second query.
App
Stock is browsed through the inventory's own category sidebar, sorted by the
localized name. Queued additions and removals appear as banners above the list
— the shared
PendingStructuralRow, extracted from the inventory — because arow drawn among the saved lines would claim a state the save does not have.
A segmented switch selects one of the two maps. That also settles which map the
ore field belongs to: the ore card appears only for the live stock, where the
number IS the purchasing power, and its line leaves the list so it is not shown
twice.
Verified against a real save
Editing ore in
m_Itemsreaches the game, and the load-time regeneration isadditive: Fisk stored 12 → 777 in game (+765); after setting 40000 → 40765
in game, the same +765. So the value is the merchant's liquidity, and it is not
re-derived from
m_DefaultItems— the failure mode the faction-hostility workwarned about does not apply here.
Also verified on a copy of a real save: a batched
setStockpair, an insert, aremoval, that add+remove is byte-identical to a no-op, and that neighbouring
merchants are untouched.
Not included
Prices. The factor lives in AngelScript (8 merchant types × 18 item tags) and
in the difficulty settings, not in the savegame.
Tests
435 Rust (incl. an integration suite that runs against the embedded start save,
so it needs no game install), 557 Flutter,
flutter analyzeclean.🤖 Generated with Claude Code
Note
Medium Risk
Changes savegame write paths and batching for trader map splices; mistakes could corrupt merchant data, but behavior mirrors existing inventory ordering rules and is covered by extensive Rust/Flutter tests.
Overview
Adds a Handel / Trade sub-tab on the character screen so merchants can be edited outside their personal inventory: live stock, restock baseline, ore (purchasing power), and add/remove stock lines.
The Rust core exposes
private.traders.list/.detailand three writes —setStock(in-place count),addItem/removeItem(map splices). Traders are keyed by array index (duplicateNonerows); sold-out lines are removed, not zeroed.private.characters.listnow includesisTraderfor list badges.The Flutter app loads traders, queues edits with stable pending keys, and mirrors core save batching (structural trader ops split from batched
setStock). UI reuses inventory-style category browsing and sharedPendingStructuralRowbanners for queued adds/removals; editing is gated when per-difficulty stock exists or the core is read-only.Reviewed by Cursor Bugbot for commit 8274c5b. Bugbot is set up for automated code reviews on this repo. Configure here.