Skip to content

Dev refactor usage#25

Merged
dcsturman merged 4 commits intomainfrom
dev-refactor-usage
Dec 22, 2025
Merged

Dev refactor usage#25
dcsturman merged 4 commits intomainfrom
dev-refactor-usage

Conversation

@dcsturman
Copy link
Owner

Refactors on how the app is used. Make dice reroll only when necessary and preserve data across browser restarts.

Don't have prices change on each skill change. Save die rolls.
Dice used to be rerolled for everything on every change. Now they are rerolled only when they have to be so you can raise and lower e.g. broker skill and get the same result.
If there isn't a specific selling world, make it so the origin is used. Its a faster use of the UI in some cases.  But we want to make this clear to the user what we're doing.
Seems most of what we are storing we want to keep across browser restarts.
@dcsturman dcsturman merged commit a0f5bd6 into main Dec 22, 2025
1 check passed
@augmentcode
Copy link

augmentcode bot commented Dec 22, 2025

🤖 Augment PR Summary

Summary: Refactors the Trade Computer flow to reduce unnecessary RNG rerolls and persist state across browser restarts.

Changes:

  • Switches Trade Computer state persistence from session storage to local storage (worlds, goods, manifest, skills).
  • Adds “saved roll” fields for trade goods pricing/quantities and passenger/freight generation so recalculations reuse prior dice results.
  • Introduces explicit “Generate” button to regenerate market-related RNG on demand instead of on every parameter change.
  • Updates manifest pricing to use destination-world trade classes when available.
  • Refactors UI components (e.g., Goods-to-sell header and SellGoodRow) to reflect destination context and read from current manifest state.
  • Reduces noisy debug logging in several modules.

Technical Notes: Persistence keys remain at :v1; state now survives reloads via use_local_storage* with JSON serialization.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 3 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

debug!("price_to_sell_rng: No trade classes provided, clearing sell price for {}", self.name);
self.sell_price = None;
self.sell_price_comment.clear();
self.sell_price_comment = "NOTHING TO SEE HERE".to_string();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When trade_classes is None, sell_price_comment is set to the literal string "NOTHING TO SEE HERE". This looks like a placeholder that can leak into UI/tooltips and be confusing; it may be better to keep the previous “clear” behavior here.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

player_broker_skill: i32,
) {
// Save the existing freight lots with their size rolls
let saved_lots = passengers.freight_lots.clone();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saved_lots is cloned from freight_lots, but freight_lots is sorted by size during generation; reusing size_roll sequentially can therefore shift rolls between Major/Minor/Incidental lots (different multipliers) and change sizes without re-rolling. That seems to undermine the goal of recalculating freight “using saved rolls” and can affect freight revenue.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

);
});

// Recalculate passengers and freight using saved rolls
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This effect only recalculates passengers when dest_world is Some and dist > 0, but it never clears available_passengers when destination is cleared or distance becomes 0. That can leave stale passengers/freight visible and used in revenue calculations.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

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 this pull request may close these issues.

1 participant