Skip to content

Reimplement native market pricing and goods trading - #216

Open
Krarilotus wants to merge 2 commits into
sourcehold:mainfrom
Krarilotus:reimplement/market-trading
Open

Reimplement native market pricing and goods trading#216
Krarilotus wants to merge 2 commits into
sourcehold:mainfrom
Krarilotus:reimplement/market-trading

Conversation

@Krarilotus

@KrarilotusKrarilotus commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

TL;DR: Reimplements the game's market prices and AI goods trading, preserving how purchases and sales change stored resources and gold. Adds a short wiki description of these rules.

The four C++03 implementations use the existing types and resolvers:

FunctionOriginal addressRaw reccmp score
GameStateStructures::getBuyPrice0x004588A0100%
GameStateStructures::getSellPrice0x00458910100%
AICState::buyGoods0x004CC00085.71%
AICState::sellGoods0x004CBFA081.82%

Prices preserve division before multiplication. Purchases charge gold only after resource storage succeeds; sales preserve the order of gold/statistics updates, resource removal and trade display. Field access uses the struct resolver's instance; member-call arguments use ptr.

Rebuilt the full OpenSHC.dll with MSVC 2005 SP1 x86, CMake 3.31.6 and RelWithDebInfo, selecting these four sources locally. Compared each function against Crusader 1.41 SHA256 3bb0a8c1e72331b3a30a5aa93ed94beca0081b476b04c1960e26d5b45387ac5a. The trade diffs are confined to resolver calls and symbolic global references; control flow, register use and field accesses match. The soft status entries use 100.0% following the review guidance for these resolver-only differences; the table above retains the raw scores.

The comparison build uses a current Windows SDK manifest tool because the legacy mt.exe crashes on this machine. Compiler and optimization settings remain the project's; no build override is included in the PR. The DLL has not been deployed or tested in-game.

Overlap: origin/reimpl/simple-functions-set-1 at 5b9da3d already contains the two price implementations, with identical expressions. The AI trade routines are additional. These price files can be dropped when that branch lands; no competing implementation is intended.

@Krarilotus
Krarilotus marked this pull request as ready for review September 5, 2026 08:57

@TheRedDaemonTheRedDaemon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi, thanks for the contribution!

Comments are on the code, also:

We have a "soft" status file status/addresses-SHC-3BB0A8C1.txt where we track the general status of the functions to avoid heavier test pipelines for the start.

The structure should be self explaining, just check some entries by searching for Reimplemented. The percentage also does not have to be too precise. If the only mismatches are the resolvers/proxies and the structure fits, saying "100.0%" is ok. If the structure still has issues, the percentage (with struct resolvers active) is ok.

Now I will also mention @gynt and add him to the review, since I will give him the final word on the wiki entry/purpose. Also, he has a lot of smaller functions in his branches and needs to check if they overlap with your MR.

Comment threadsrc/OpenSHC/Game/GameStateStructures/getSellPrice.cpp Outdated
Comment threadsrc/OpenSHC/Game/GameStateStructures/getBuyPrice.cpp Outdated
Comment threadsrc/OpenSHC/AI/AICState/sellGoods.cpp Outdated
Comment threaddocs/wiki.rst
The Game itself
------------------
- :doc:`Load balancing of the core game engine <wiki/load-balancing-table>`
- :doc:`Market pricing and goods trading <wiki/market-trading>`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you replace something here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

No existing entry was replaced: the diff adds only the market-trading link below the load-balancing page. The rest of the index is unchanged.

@gyntgyntSep 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it is unclear that this article will be about AI. Please add a AI subsection and add this entry to it with AI in the title. And make sure the .md file refers to AI market behavior explicitly.

Comment threaddocs/wiki/market-trading.md Outdated
Comment threaddocs/wiki/market-trading.md Outdated
Comment threaddocs/wiki/market-trading.md Outdated
@Krarilotus

Copy link
Copy Markdown
ContributorAuthor

Updated in 9edc084: field access now uses instance, the redundant comments are removed, and the wiki describes vanilla trading behavior without build-process details. I also added the four soft status entries. Fresh RelWithDebInfo DLL comparisons give 100% for both price functions; the trade routines retain the resolver-only differences documented in the PR, so their status follows the suggested 100.0% convention.

I checked Gynt's reimpl/simple-functions-set-1 branch as well: it already has both price expressions. The AI buy/sell routines are additional; I have noted the overlap in the description so the price files can be dropped when that branch lands.

Comment threaddocs/wiki.rst
The Game itself
------------------
- :doc:`Load balancing of the core game engine <wiki/load-balancing-table>`
- :doc:`Market pricing and goods trading <wiki/market-trading>`

@gyntgyntSep 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it is unclear that this article will be about AI. Please add a AI subsection and add this entry to it with AI in the title. And make sure the .md file refers to AI market behavior explicitly.

@TheRedDaemonTheRedDaemon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good from my side.

Sign up for freeto 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.

3 participants

@Krarilotus@gynt@TheRedDaemon