Trading design - rebaserHEAD/NexusForever GitHub Wiki

Player-to-player trading between two characters standing together: an invite, offers of items and credits on both sides, and an exchange written to the database for both characters at once. It's built on branch player-trading and compiles, but it hasn't been run in game yet, so nothing on stable handles trading. The marketplace, mail and item binding are out of scope.

[!IMPORTANT] Trading needs the base entitlements from Account entitlements. Without EconomyParticipation the client won't open the trade window, and the server refuses the request too.

The idea

In three sentences

The server holds each trade from invite to exchange and echoes every accepted change to both players, because the client applies nothing on its own. Once both have committed, it waits until neither player has a save in flight, holds both players' saves, moves the same item objects and the credits, and writes both characters in one transaction. If that write fails, the exchange is undone in memory before the holds are released.

Why

• It's open ground. Upstream has all 13 trade messages modelled (#492) and nothing that handles them, on every branch. It needs no game tables, no schema change and no combat.
• It's checkable here. With the base entitlements in place the client already opens the window and sends the first request, and two clients run side by side on one machine.
• The stakes are the economy. A trade must never duplicate or lose an item or a credit, including when the server stops mid-trade. That's what shapes the save.

Decisions

Locked

Decision	Why
One global ITradeManager, one ITrade per pair, made through a factory	Upstream's MatchManager shape. Nothing about a trade is saved per character, and the busy check needs both sides anyway
Trade state is touched only on the world thread, outside the map update	Message handlers and logout run one after another in the network update, and the manager ticks after the maps. Maps update in parallel, so nothing in a map update calls in. No locks are needed
Echo, don't trust	The client applies no change itself. Every accepted add, remove, credit amount and commit goes to both players, the owner included; a refused one gets no answer
Every end sends a result with the cancel flag to each connected player	It's the only thing that closes the client's trade window
Offers are re-checked, not locked	Locking an offered item would mean refusing it on about a dozen upstream paths, some of which refuse by disconnecting, and the server changes stacks on its own anyway. Each offer is checked against the live bag every tick, at the final commit and again just before the exchange; one that moved, split or changed size is withdrawn on both screens
What both committed to is what moves	Any change to either offer clears both commits. A commit that arrives within a second of a change is ignored, since it may have been sent before the change arrived
One transaction for both characters	The exchange waits until neither player is saving, holds both players' saves, applies, and writes both characters with one context and one SaveChangesAsync. Holding a save is its own change, below
Undo in memory on failure, before the holds are released	Otherwise the next periodic save of either player would write half the trade
The same objects move	Items leave one bag and join the other as the same object, and credits are subtracted and added in one call. Nothing is created or destroyed
Invites expire after 60 seconds	The client has no expiry, and its report button hides an invite without declining it
A trade ends on logout, a teleport, a loading screen, losing sight of the partner, death, combat, a vehicle or mount, or losing the right to trade	Logout ends it from Player.OnLogout, like matches; the rest are checked each tick. Distance is checked only at the start, after which sight is the leash
Completed trades are logged at Info	It's the only record of items and credits changing hands at default log levels. Everything else logs at Trace

Holding a player's save

A trade can't write two characters together if either can start a save of its own. So player-save-hold changes Player itself, and stands as its own pull request:

• Saves no longer overlap. A save requested while another is in flight waits for it. Two saves in flight snapshot at different times and can commit in either order, so an older one could write its absolute values over a newer one.
• IPlayer.HoldSave() returns a handle. While any handle is held, the player's packets aren't handled, the save timer is paused (not reset, since played time comes from it), and a requested save waits for the release.
• IPlayer.IsSaving says whether a save is in flight or held.

Open

• Item binding. The server has no per-item bound state yet, so the client's own check is the only bind rule. A modified client could trade a bound item. Binding belongs in its own change, shared with mail, auctions and the guild bank.
• Mounts. A mount counts as a vehicle, so trading while mounted is refused. If the client allows it, mounts should be exempt.
• What the player who cancels sees. They're sent PlayerCanceled, which prints "Trade failed." in their own chat. If that reads badly, another code can be used.
• Ending on combat. Chosen so a live trade always meets the client's own start conditions. It's one condition to drop if it annoys.
• The one-second commit lockout is sized to a round trip. A very fast clicker sees one commit ignored.

Rejected

Option	What killed it
A per-character trade manager on Player	Several upstream edits for state that's never saved, and the busy check needs both players anyway
Ending trades from death or map-removal hooks	Those run in the parallel map update, so they'd need locks. Logout is different: it only runs from the network update
Locking offered items	A dozen paths to change, some that refuse by disconnecting the client, and a re-check is still needed for the server's own stack changes
Writing the trade on the world thread and waiting	It stalls the whole world for a database round trip per trade, and leaves the overlapping-save race in place
Letting each player's own save write their half	A stop between the two halves duplicates credits or loses items, and an older save still in flight can write over the trade

Status

Verification status

Claim	Basis
It compiles	Built clean on player-trading
Restructuring into upstream's shape changed no behaviour	Reviewed path by path against the first build: every state change and result sent is the same, apart from logout now ending the trade from OnLogout
Logout runs on the world thread, outside the map update	Read from the code: only the client's logout confirmation and a dropped connection finish a logout, both in the network update
The message corrections: item guid and credits read as 64 bits, the unit id written before the credits	Read from the client, not yet seen on the wire
Anything in game	Not run. A two-client test is planned: decline, cancel, a completed trade, a change clearing commits, withdrawal, the commit lockout, a full bag, a disconnect, a teleport, a mount and a failed save

Open questions

• Whether the client frees its trade object on a world change. Every end already sends a code with no side effect on the client, and the teleport test checks that a new trade opens afterwards.
• The last five fields of ServerP2PTradeUpdateItem are sent as zeros, matching what the bag shows for the server's items.
• Refusing a busy target needs a third player to check.

Upstream surface

Three pull requests, each building on the one before. None is open yet; they wait on the in-game test.

Branch	What	Builds on
p2p-trade-messages	The message model corrections	game_rework
player-save-hold	Saves no longer overlap, and can be held	game_rework
player-trading	The trade manager and eight handlers	Both of the above; Account entitlements for anyone to trade in play

The save change is the one upstream may push back on: it touches Player.Save, which every player goes through.

Where the code lives

Piece	Path
Trade manager	Source/NexusForever.Game/Trade/TradeManager.cs
A trade and its two sides	Source/NexusForever.Game/Trade/Trade.cs, TradeParticipant.cs
Interfaces and offered items	Source/NexusForever.Game.Abstract/Trade/
States	Source/NexusForever.Game.Static/Trade/TradeState.cs
Save holds	Source/NexusForever.Game/Entity/Player.cs
Handlers	Source/NexusForever.WorldServer/Network/Message/Handler/Trade/
Message models	Source/NexusForever.Network.World/Message/Model/ClientP2PTrading*.cs, ServerP2PTrade*.cs