cap buy and sell orders by resource and lords balance #1714
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Preparing PR description... |
|
Preparing review... |
![mentatbot[bot]](https://avatars.githubusercontent.com/in/837875?s=60&v=4){kind=small}
There was a problem hiding this comment.
The changes effectively address the issue of capping buy and sell orders based on available balances. The implementation is well-thought-out, using ProductionManager for accurate balance calculations and introducing balance ratios to limit input values. The code is clean, consistent, and follows good React practices with the use of useMemo and useEffect. The updates to the 'Max' button and NumberInput component ensure that users can't exceed their available balances. Overall, this is a solid implementation that should resolve the issue while maintaining good performance and user experience.
Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!
| @@ -154,6 +155,8 @@ const MarketOrders = ({ | |||
| offers: MarketInterface[]; | |||
| isOwnStructureInBattle: boolean; | |||
| }) => { | |||
| const [updateBalance, setUpdateBalance] = useState(false); | |||
There was a problem hiding this comment.
Good addition of the updateBalance state. This will help trigger re-renders when balances change.
| systemCalls: { cancel_order, accept_partial_order }, | ||
| }, | ||
| } = useDojo(); | ||
|
|
There was a problem hiding this comment.
Nice use of ProductionManager to get accurate balances for Lords and resources.
| @@ -282,6 +298,28 @@ | |||
|
|
|||
| const currentDefaultTick = useUIStore((state) => state.currentDefaultTick); | |||
|
|
|||
| const resourceBalanceRatio = useMemo( | |||
There was a problem hiding this comment.
Good implementation of resourceBalanceRatio and lordsBalanceRatio to cap the input value based on available balances.
| : (offer.takerGets[0].amount / EternumGlobalConfig.resources.resourcePrecision) * lordsBalanceRatio; | ||
| }); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
The useEffect hook is a good addition to update the input value when balance ratios change.
| @@ -336,6 +374,7 @@ | |||
| } catch (error) { | |||
| console.error("Failed to accept order", error); | |||
| } finally { | |||
| setUpdateBalance(!updateBalance); | |||
There was a problem hiding this comment.
Good practice to update the balance after a successful transaction.
| @@ -418,11 +457,17 @@ | |||
| value={inputValue} | |||
| className="w-full col-span-3" | |||
| onChange={setInputValue} | |||
| max={getsDisplayNumber / EternumGlobalConfig.resources.resourcePrecision} | |||
| max={ | |||
There was a problem hiding this comment.
Nice update to the max prop of the NumberInput component to respect the balance ratios.
| /> | ||
| <Button | ||
| onClick={() => { | ||
| setInputValue(getsDisplayNumber / EternumGlobalConfig.resources.resourcePrecision); | ||
| setInputValue( |
There was a problem hiding this comment.
Good update to the 'Max' button functionality to respect the new balance limits.
Fixes #1673