Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update FillPlayerTradeModal.tsx #403

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update FillPlayerTradeModal.tsx #403

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
231 changes: 199 additions & 32 deletions src/scripts/ui/FillPlayerTradeModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from "../../../shared/utilities/Helper";
import { L, t } from "../../../shared/utilities/i18n";
import { useGameState } from "../Global";
import { client, usePlayerMap, useTrades } from "../rpc/RPCClient";
import { client, usePlayerMap, useTrades, addSystemMessage } from "../rpc/RPCClient";
import { findPath, findUserOnMap, getMyMapXy } from "../scenes/PathFinder";
import { playError, playKaching } from "../visuals/Sound";
import { hideModal, showToast } from "./GlobalModal";
Expand All @@ -35,6 +35,12 @@ export function FillPlayerTradeModal({ tradeId, xy }: { tradeId: string; xy?: Ti
const allTradeBuildings = Tick.current.playerTradeBuildings;
const [fills, setFills] = useState(new Map<Tile, number>());

const DEFAULT_HINTS = "greedy";
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure why DEFAULT_HINTS is needed here. Having an opaque variable should generally be avoided


// added, to keep buildings in the order of best-ish subtrade candidates
// filled later a few lines below "Array.from(allTradeBuildings.entries())"
var allTradeBuildingsSorted = new Map();

useEffect(() => {
if (!trade) {
return;
Expand Down Expand Up @@ -82,47 +88,196 @@ export function FillPlayerTradeModal({ tradeId, xy }: { tradeId: string; xy?: Ti
return true;
};

const calculateMaxFill = () => {
// major changes 2025-02-xx
// param "hint" can be "greedy" or "fast" to tweak some limits
// param "maxError" reserved for future use, to set tolerance for fluctuating, erronous or invalid values
const calculateMaxFill = (hints = "auto", maxError = 0.03) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, hints should not be included. Also the magic number 0.03 should not be included

const result = new Map<Tile, number>();
let amountLeft = trade.buyAmount;
for (const xy of allTradeBuildings.keys()) {
const amount = getMaxFill(xy);
if (amount <= 0) {
// Do nothing
} else if (amountLeft > amount) {
result.set(xy, amount);
amountLeft -= amount;

// enable some tweaks only in large trades
const isLargeTrade = trade.buyAmount >= 10000000; // 10 million
Copy link
Contributor

Choose a reason for hiding this comment

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

10000000 magic number should be avoided


// does it really help?
// no, no longer needed with later tweaks as of 2025-02-xx
// if(isLargeTrade) {
// amountLeft = amountLeft * 0.90;
// }

// of the goods we're sending to them. first is alias, second is calculated later
const totalAmountTheyWant = trade.buyAmount;
let totalAmountWeHave = 0;

let amountLeftToSend = totalAmountTheyWant;

// manual index for loop, note it's 1-based
var currentSubtradeIndex = 0;

// don't ignore subtrades below that number
// mostly for the benefit of players with heavily clogged up storage
var minSubtrades = 10;

// max number of subtrades allowed. 200 seems reasonable as of b569/England
var maxSubtrades = 200;

if (typeof hints === "string" && hints.includes("greedy")) {
// greedy is default now, so no changes here
}

if (typeof hints === "string" && hints.includes("fast")) {
// note since subtrades are sorted 20 is usually enough to fill majority of the trade
maxSubtrades = 20;
}

// pre-calc loop
for (const xy of allTradeBuildingsSorted.keys()) {
// at this moment, in trade buildings
totalAmountWeHave += getMaxFill(xy);
}

// addSystemMessage(`totalAmountWeHave=${formatNumber(totalAmountWeHave)} / `+
// `totalAmountTheyWant=${formatNumber(totalAmountTheyWant)}`);

// main loop
for (const xy of allTradeBuildingsSorted.keys()) {
// hard limit, ignore further possible subtrades
if (currentSubtradeIndex >= maxSubtrades) {
break;
}

currentSubtradeIndex++; // 1-based

// IMPORTANT
// do only partial fills
// does not affect the total amount as long as there is some free space
// TODO: allow tweaking with errMargin, maybe like getMaxFill(xy) * (1 - errMargin)
// would require more testing. 0.90 seems good
let subtradeAmount = getMaxFill(xy) * 0.9;

// we want to ignore trades that are zero or close to zero,
// as they introduce needless delays in client and load on server
var isSubtradeTooSmall = subtradeAmount < totalAmountWeHave / 1000;

// addSystemMessage(`cS=${currentSubtrade} / maxS=${maxSubtrades}, `+
// `pA=${formatNumber(partialAmount)} / aL=${formatNumber(amountLeft)} / `+
// ` tAWH=${formatNumber(totalAmountWeHave)} / tATW=${formatNumber(totalAmountTheyWant)}, `+
// `subtradeTooSmall=${subtradeTooSmall}`);

// use 2nd subtrade to expose mod's version
// this 'wastes' a single subtrade but is the easiest way to
// advertise which version is used with no additions elsewhere
// not needed in production version
// if (currentSubtrade == 2) {
// if (partialAmount > MODDEDCLIENT_VER) {
// partialAmount = MODDEDCLIENT_VER;
// }
// }

// tweaks that should run only between minSubtrades to maxSubtrades
if (currentSubtradeIndex > minSubtrades) {
if (isSubtradeTooSmall) {
// ignore - prevent from sending to the server - tiny subtrades,
// greatly reduces waste both on client and server
// during testing often 50-80% of subtrades were these near-zero ones
continue;
}
}

if (subtradeAmount <= 0) {
// redundant now? leave it for now
// will work with first minSubtrades too if they're zero or negative but not if they're tiny
// do nothing
} else if (amountLeftToSend > subtradeAmount) {
result.set(xy, subtradeAmount);
amountLeftToSend -= subtradeAmount;
} else {
result.set(xy, amountLeft);
amountLeft = 0;
result.set(xy, amountLeftToSend);
amountLeftToSend = 0;
break;
}
}
return result;
};

const doFill = async (fills: Map<Tile, number>) => {
if (!fillsAreValid(fills) || !hasValidPath()) {
// major changes 2025-02-xx
// param "hint" can be "greedy" or "fast" to tweak some limits
// param "maxError" reserved for future use, to set tolerance for fluctuating, erronous or invalid values
const doFill = async (fills: Map<Tile, number>, hints = "auto", maxError = 0.03) => {
// split error checking so it's easier to debug
if (!hasValidPath()) {
addSystemMessage("hasValidPath=false");
showToast(t(L.OperationNotAllowedError));
playError();
return;
}
if (!fillsHaveEnoughResource(fills)) {
addSystemMessage("fillsHaveEnoughResource=false");
showToast(t(L.OperationNotAllowedError));
playError();
return;
}
if (!fillsHaveEnoughStorage(fills)) {
addSystemMessage("fillsHaveEnoughStorage=false");
showToast(t(L.OperationNotAllowedError));
playError();
return;
}

let totalFillAmount = getTotalFillAmount(fills);
if (!(totalFillAmount > 0)) {
// happens rarely, but usually no less than once / 15 minutes during aggressive trading
let str1 = `totalFillAmount=${totalFillAmount} is negative. This shouldn't happen. Trying to proceed anyway.`;
addSystemMessage(str1);

// this is partially recoverable, we do not want to fail here
//return;
}

if (!(totalFillAmount <= trade.buyAmount)) {
// needs more testing
let str1 = `totalFillAmount=${totalFillAmount} is greater than trade.buyAmount=${trade.buyAmount}. This shouldn't happen. Trying to proceed anyway.`;
addSystemMessage(str1);

// // seems partially recoverable. needs more testing.
//ct(h(d.OperationNotAllowedError)), ze();
//return;
}

// todo: localize?
showToast("Filling trades, please wait 5-20 sec...");

let total = 0;
let success = 0;
let fillAmount = 0;
let receivedAmount = 0;

const errors: string[] = [];
const queue: Array<{ amount: number; rollback: () => void; tile: Tile }> = [];

const subtradeQueue: Array<{ amount: number; rollback: () => void; tile: Tile }> = [];

// 1st loop, queue subtrades
for (const [tile, amount] of fills) {
if (amount <= 0) continue;
// We reserve the amount first, otherwise resource might go negative if a player
// clicks really fast
++total;

// shouldn't this be in the 2nd loop?
//++total;

const r = deductResourceFrom(trade.buyResource, amount, [tile], gs);
queue.push({ amount: r.amount, rollback: r.rollback, tile });
subtradeQueue.push({ amount: r.amount, rollback: r.rollback, tile });
}
for (const r of queue) {

// 2nd loop, process queued subtrades
for (const r of subtradeQueue) {
try {
total++;

// todo: localize
let tradeStr = `${total} / ${fills.size}`;
let resourceStr = `${formatNumber(fillAmount)} ${trade.buyResource}`;
let str1 = `Filling trades ${tradeStr}, sending: ${resourceStr}...`;
Copy link
Contributor

Choose a reason for hiding this comment

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

All strings should be localized

showToast(str1);

const result = await client.fillTrade({
id: trade.id,
amount: r.amount,
Expand All @@ -149,16 +304,21 @@ export function FillPlayerTradeModal({ tradeId, xy }: { tradeId: string; xy?: Ti
}
if (success > 0) {
playKaching();
errors.unshift(
t(L.PlayerTradeFillSuccessV2, {
success,
total,
fillAmount: formatNumber(fillAmount),
fillResource: Config.Resource[trade.buyResource].name(),
receivedAmount: formatNumber(receivedAmount),
receivedResource: Config.Resource[trade.sellResource].name(),
}),
);

let str1 = t(L.PlayerTradeFillSuccessV2, {
success,
total,
fillAmount: formatNumber(fillAmount),
fillResource: Config.Resource[trade.buyResource].name(),
receivedAmount: formatNumber(receivedAmount),
receivedResource: Config.Resource[trade.sellResource].name(),
});
errors.unshift(str1);

// todo: do we want it?
// some testers liked having it in chat window, some didn't
// addSystemMessage(str1);

const eic = Tick.current.specialBuildings.get("EastIndiaCompany");
if (eic) {
safeAdd(
Expand Down Expand Up @@ -277,6 +437,9 @@ export function FillPlayerTradeModal({ tradeId, xy }: { tradeId: string; xy?: Ti
(b.resources[trade.buyResource] ?? 0) - (a.resources[trade.buyResource] ?? 0),
)
.map(([xy, building]) => {
// save the order for later use in other places
allTradeBuildingsSorted.set(xy, building);

const storage = getStorageFor(xy, gs);
return (
<tr key={xy}>
Expand All @@ -292,7 +455,7 @@ export function FillPlayerTradeModal({ tradeId, xy }: { tradeId: string; xy?: Ti
className="text-right text-small text-link"
onClick={() => {
setFills((old) => {
old.set(xy, getMaxFill(xy));
old.set(xy, getMaxFill(xy, DEFAULT_HINTS));
return new Map(fills);
});
}}
Expand Down Expand Up @@ -457,9 +620,9 @@ export function FillPlayerTradeModal({ tradeId, xy }: { tradeId: string; xy?: Ti
<div className="f1"></div>
<button
onClick={() => {
const fills = calculateMaxFill();
const fills = calculateMaxFill(DEFAULT_HINTS);
if (fills.size > 0) {
doFill(fills);
doFill(fills, DEFAULT_HINTS);
} else {
playError();
showToast(t(L.PlayerTradeNoFillBecauseOfResources));
Expand All @@ -470,7 +633,11 @@ export function FillPlayerTradeModal({ tradeId, xy }: { tradeId: string; xy?: Ti
{t(L.PlayerTradeFillAmountMaxV2)}
</button>
<div style={{ width: "6px" }}></div>
<button className="text-strong" disabled={!fillsAreValid(fills)} onClick={() => doFill(fills)}>
<button
className="text-strong"
disabled={!fillsAreValid(fills)}
onClick={() => doFill(fills, DEFAULT_HINTS)}
>
{t(L.PlayerTradeFillTradeButton)}
</button>
</div>
Expand Down
Loading