Skip to content

Conversation

@ErwinsExpertise
Copy link
Collaborator

This pull request implements the core functionality for player shops, allowing players to create, manage, and interact with personal shops in the game. It introduces a new shopRoom type, handles shop-related operations (such as adding, buying, and removing items), and updates the packet protocol to support shop features. Additionally, it generalizes the display logic for rooms and introduces a new interface for game box display.

The most important changes are:

Player Shop System Implementation:

  • Added a new shopRoom type with logic for creating shops, managing players, adding/removing items, handling purchases, and shop closure. This includes the shopItem struct and methods such as addPlayer, removePlayer, addItem, buyItem, and closeShop.
  • Integrated player shop operations into the main room handler, including creating shops, opening/closing shops, adding/buying/removing shop items, and sending appropriate packets to clients. [1] [2] [3] [4]
  • Added new packet constructors for shop actions, such as refreshing shop items, notifying item sales, removing items, and sending shop operation results.

Room and Display Logic Generalization:

  • Introduced the boxDisplayer interface to generalize how rooms display themselves on the game map, and updated the room pool logic to use this interface instead of the previous gameRoomer type. [1] [2] [3] [4]
  • Implemented the displayBytes method for shopRoom to support the new interface and ensure proper shop display on the map.

Protocol and Constants Updates:

  • Updated the packet protocol for room/shop features, including new constants for shop-related packet types, shop limits, and packet field values. [1] [2]
  • Modified packetRoomShowWindow and related packet functions to handle player shop-specific data and ensure compatibility with the new shop system. [1] [2]

These changes collectively enable the creation and management of player shops, allowing for item trading between players in a controlled room environment.

@ErwinsExpertise ErwinsExpertise requested a review from Hucaru January 1, 2026 22:25
@ErwinsExpertise ErwinsExpertise self-assigned this Jan 1, 2026
@ErwinsExpertise ErwinsExpertise added enhancement feature Ticket for discussing a feature and it's implementation labels Jan 1, 2026
@ErwinsExpertise
Copy link
Collaborator Author

This PR only is for player shops. I have not implemented hired merchants.

@Hucaru
Copy link
Owner

Hucaru commented Jan 6, 2026

Why are player shops being stored in the database? These only need to persist whilst a player is online. Only hired merchants need to persist.

The player struct could be modified to have a shopItems []*Item (fixed size array prefered over slice if possible) entry where salePrice and saleAmount are added to the item struct. This way if a player disconnects there is nothing to manage apart from removing the room from the pool.

I can't remember how quantites are managed in player shops but if the client supports selling more than 1 of an item then at the point of sale the saleAmount and amount fields in the item can both be modified and the item can then be marked as dirty/saved.

@ErwinsExpertise
Copy link
Collaborator Author

Why are player shops being stored in the database? These only need to persist whilst a player is online. Only hired merchants need to persist.

The player struct could be modified to have a shopItems []*Item (fixed size array prefered over slice if possible) entry where salePrice and saleAmount are added to the item struct. This way if a player disconnects there is nothing to manage apart from removing the room from the pool.

I can't remember how quantites are managed in player shops but if the client supports selling more than 1 of an item then at the point of sale the saleAmount and amount fields in the item can both be modified and the item can then be marked as dirty/saved.

I went with DB escrow because once an item is listed I’m actually removing it from the character’s inventory, and keeping that removed inventory purely in memory means a crash can lose items. I originally tried the simpler “visual remove / add back later” approach and ran into consistency problems: the client inventory UI and the server’s idea of slots/stacks would drift, and getting items to reliably reappear without desyncs or edge-case glitches was messy. Escrow keeps things authoritative and predictable by making the item live in exactly one place at a time (inventory or escrow) and gives a clean restore path on login. It also sets things up for hired merchants later since they need persistence and can reuse the same escrow/restore mechanism instead of introducing a separate system.

@Hucaru
Copy link
Owner

Hucaru commented Jan 6, 2026

I went with DB escrow because once an item is listed I’m actually removing it from the character’s inventory, and keeping that removed inventory purely in memory means a crash can lose items. I originally tried the simpler “visual remove / add back later” approach and ran into consistency problems: the client inventory UI and the server’s idea of slots/stacks would drift, and getting items to reliably reappear without desyncs or edge-case glitches was messy. Escrow keeps things authoritative and predictable by making the item live in exactly one place at a time (inventory or escrow) and gives a clean restore path on login. It also sets things up for hired merchants later since they need persistence and can reuse the same escrow/restore mechanism instead of introducing a separate system.

This should only be the case if a save isn't performed at the point of sale. When an item is bought a save should occur for both parties, to guarantee no desync's. If the seller disconnects and relogs then they will just load in the inventory from db which will be pre-shop creation minus any sales.

When a player makes a shop and places an item into the shop list what happens in the client UI? Does the item disappear from it's item slot like a trade window? Are you able to attach a screenshot? You should be able to treat this similar to a trade room with only 1 side having items.

@ErwinsExpertise
Copy link
Collaborator Author

ErwinsExpertise commented Jan 7, 2026

I went with DB escrow because once an item is listed I’m actually removing it from the character’s inventory, and keeping that removed inventory purely in memory means a crash can lose items. I originally tried the simpler “visual remove / add back later” approach and ran into consistency problems: the client inventory UI and the server’s idea of slots/stacks would drift, and getting items to reliably reappear without desyncs or edge-case glitches was messy. Escrow keeps things authoritative and predictable by making the item live in exactly one place at a time (inventory or escrow) and gives a clean restore path on login. It also sets things up for hired merchants later since they need persistence and can reuse the same escrow/restore mechanism instead of introducing a separate system.

This should only be the case if a save isn't performed at the point of sale. When an item is bought a save should occur for both parties, to guarantee no desync's. If the seller disconnects and relogs then they will just load in the inventory from db which will be pre-shop creation minus any sales.

When a player makes a shop and places an item into the shop list what happens in the client UI? Does the item disappear from it's item slot like a trade window? Are you able to attach a screenshot? You should be able to treat this similar to a trade room with only 1 side having items.

I get the just save on sale argument, but that doesn’t address what was actually going wrong for me. The hard part wasn’t persisting the buyer/seller at the moment of purchase, it was trying to model a shop as “the item is still in the owner’s inventory, but also listed somewhere else” and then relying on UI tricks or a trade-like one-sided flow to keep everything consistent. Once you do that you’re forced into either faking inventory state (and dealing with slot/stack desyncs when things merge/split, partial quantities, etc.) or implementing a real lock/reserve system so the listed item can’t be moved/dropped/traded/consumed while it’s listed. That lock approach sounds simple but it spreads everywhere: every inventory-changing path has to check and enforce it, and if you miss one you’ve created a dupe/desync. The DB escrow approach keeps it contained and deterministic: listing is a real server-side state change (item is no longer in inventory while listed), so you don’t need global locking rules, restore is straightforward, and it’s also the same primitive hired merchants will need anyways. So this isn’t persisting player shops, it’s persisting removed inventory safely and reusing the same path for merchant persistence later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement feature Ticket for discussing a feature and it's implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants