-
Notifications
You must be signed in to change notification settings - Fork 51
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.
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
optimistic #2220
optimistic #2220
Changes from 7 commits
bc2e635
68d7f26
fde3ffa
888f251
5914ac5
f64b391
626849f
c34b248
ea8eb7a
ca8681c
4722878
907bede
2149140
052d71c
0145224
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,97 @@ | ||
import { DojoResult } from "@/hooks/context/DojoContext"; | ||
import { ID } from "@bibliothecadao/eternum"; | ||
import { ID, ResourcesIds } from "@bibliothecadao/eternum"; | ||
import { getComponentValue } from "@dojoengine/recs"; | ||
import { getEntityIdFromKeys } from "@dojoengine/utils"; | ||
import { uuid } from "@latticexyz/utils"; | ||
import { ResourceManager } from "./ResourceManager"; | ||
|
||
interface Troops { | ||
[ResourcesIds.Paladin]: number; | ||
[ResourcesIds.Crossbowman]: number; | ||
[ResourcesIds.Knight]: number; | ||
} | ||
|
||
export class ArmyManager { | ||
constructor(private dojo: DojoResult) {} | ||
|
||
async deleteArmy(armyId: ID): Promise<void> { | ||
await this.dojo.setup.systemCalls | ||
.delete_army({ | ||
signer: this.dojo.account.account, | ||
army_id: armyId, | ||
}) | ||
.then(() => { | ||
this.dojo.network.world.deleteEntity(getEntityIdFromKeys([BigInt(armyId)])); | ||
}); | ||
private readonly realmEntityId: number; | ||
|
||
constructor( | ||
private readonly dojo: DojoResult, | ||
private readonly armyEntityId: ID, | ||
) { | ||
this.realmEntityId = this.getRealmEntityId(); | ||
} | ||
|
||
private getRealmEntityId(): number { | ||
return ( | ||
getComponentValue(this.dojo.setup.components.EntityOwner, getEntityIdFromKeys([BigInt(this.armyEntityId)])) | ||
?.entity_owner_id || 0 | ||
); | ||
} | ||
|
||
private _updateResourceBalances(overrideId: string, troops: Troops): void { | ||
Object.entries(troops).forEach(([troopType, amount]) => { | ||
const resourceManager = new ResourceManager( | ||
this.dojo.setup, | ||
this.realmEntityId, | ||
Number(troopType) as ResourcesIds, | ||
); | ||
resourceManager.optimisticResourceUpdate(overrideId, -BigInt(amount)); | ||
}); | ||
} | ||
|
||
private _updateArmyTroops(overrideId: string, army: any, troops: Troops): void { | ||
this.dojo.setup.components.Army.addOverride(overrideId, { | ||
entity: getEntityIdFromKeys([BigInt(this.armyEntityId)]), | ||
value: { | ||
...army, | ||
troops: { | ||
knight_count: BigInt(army.troops.knight_count) + BigInt(troops[ResourcesIds.Knight]), | ||
crossbowman_count: BigInt(army.troops.crossbowman_count) + BigInt(troops[ResourcesIds.Crossbowman]), | ||
paladin_count: BigInt(army.troops.paladin_count) + BigInt(troops[ResourcesIds.Paladin]), | ||
Comment on lines
+48
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check for Potential Integer Overflows When updating troop counts in Consider adding checks to prevent overflows: +// Check for potential overflows
+const newKnightCount = BigInt(army.troops.knight_count) + BigInt(troops[ResourcesIds.Knight]);
+if (newKnightCount > MAX_SAFE_INTEGER) {
+ throw new Error('Knight count exceeds maximum safe integer.');
+}
|
||
}, | ||
}, | ||
}); | ||
} | ||
|
||
private _optimisticAddTroops(overrideId: string, troops: Troops): void { | ||
const entity = getEntityIdFromKeys([BigInt(this.armyEntityId)]); | ||
const army = getComponentValue(this.dojo.setup.components.Army, entity); | ||
|
||
if (!army) return; | ||
|
||
this._updateResourceBalances(overrideId, troops); | ||
this._updateArmyTroops(overrideId, army, troops); | ||
} | ||
|
||
public addTroops(troops: Troops): void { | ||
this.dojo.setup.systemCalls.army_buy_troops({ | ||
signer: this.dojo.account.account, | ||
payer_id: this.realmEntityId, | ||
army_id: this.armyEntityId, | ||
troops: { | ||
knight_count: BigInt(troops[ResourcesIds.Knight]), | ||
crossbowman_count: BigInt(troops[ResourcesIds.Crossbowman]), | ||
paladin_count: BigInt(troops[ResourcesIds.Paladin]), | ||
}, | ||
}); | ||
|
||
this._optimisticAddTroops(uuid(), troops); | ||
} | ||
Comment on lines
+66
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Handle Asynchronous System Call in The Update the method to handle the asynchronous operation: -public addTroops(troops: Troops): void {
+public async addTroops(troops: Troops): Promise<void> {
try {
await this.dojo.setup.systemCalls.army_buy_troops({
// parameters...
});
this._optimisticAddTroops(uuid(), troops);
} catch (error) {
// Handle error appropriately
console.error('Error adding troops:', error);
}
}
|
||
|
||
public createArmy(structureEntityId: bigint, isDefensive: boolean): void { | ||
this.dojo.setup.systemCalls.create_army({ | ||
signer: this.dojo.account.account, | ||
is_defensive_army: isDefensive, | ||
army_owner_id: structureEntityId, | ||
}); | ||
} | ||
|
||
public async deleteArmy(armyId: ID): Promise<void> { | ||
await this.dojo.setup.systemCalls.delete_army({ | ||
signer: this.dojo.account.account, | ||
army_id: armyId, | ||
}); | ||
|
||
this.dojo.network.world.deleteEntity(getEntityIdFromKeys([BigInt(armyId)])); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -12,6 +12,7 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ContractAddress, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ID, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ResourcesIds, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
TravelTypes, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
getDirectionBetweenAdjacentHexes, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
getNeighborHexes, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} from "@bibliothecadao/eternum"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -220,6 +221,18 @@ export class ArmyMovementManager { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private readonly _optimisticCapacityUpdate = (overrideId: string, capacity: number) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const currentWeight = getComponentValue(this.setup.components.Weight, this.entity); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Weight.addOverride(overrideId, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
entity: this.entity, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
value: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
entity_id: this.entityId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
value: (currentWeight?.value || 0n) + BigInt(capacity), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private readonly _optimisticTileUpdate = (overrideId: string, col: number, row: number) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const entity = getEntityIdFromKeys([BigInt(col), BigInt(row)]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -252,6 +265,12 @@ export class ArmyMovementManager { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._optimisticStaminaUpdate(overrideId, configManager.getExploreStaminaCost(), currentArmiesTick); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._optimisticTileUpdate(overrideId, col, row); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._optimisticPositionUpdate(overrideId, col, row); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._optimisticCapacityUpdate( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
overrideId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// all resources you can find have the same weight as wood | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
configManager.getExploreReward() * configManager.getResourceWeight(ResourcesIds.Wood), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._optimisticFoodCosts(overrideId, TravelTypes.Explore); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return overrideId; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -277,18 +296,21 @@ export class ArmyMovementManager { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
signer: useAccountStore.getState().account!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.catch((e) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Position.removeOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Tile.removeOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// remove all visual overrides only when the action fails | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._removeVisualOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._removeNonVisualOverrides(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.then(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Stamina.removeOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// remove all non visual overrides | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._removeNonVisualOverrides(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private readonly _optimisticTravelHex = (col: number, row: number, pathLength: number, currentArmiesTick: number) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const overrideId = uuid(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._optimisticStaminaUpdate(overrideId, configManager.getTravelStaminaCost() * pathLength, currentArmiesTick); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._optimisticFoodCosts(overrideId, TravelTypes.Travel); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Position.addOverride(overrideId, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
entity: this.entity, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -301,6 +323,32 @@ export class ArmyMovementManager { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return overrideId; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// only remove visual overrides (linked to models on world map) when the action fails | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private readonly _removeVisualOverride = (overrideId: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Tile.removeOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Position.removeOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// you can remove all non visual overrides when the action fails or succeeds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private readonly _removeNonVisualOverrides = (overrideId: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Stamina.removeOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Resource.removeOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Weight.removeOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private readonly _optimisticFoodCosts = (overrideId: string, travelType: TravelTypes) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const entityArmy = getComponentValue(this.setup.components.Army, this.entity); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let costs = { wheatPayAmount: 0, fishPayAmount: 0 }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (travelType === TravelTypes.Explore) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
costs = computeExploreFoodCosts(entityArmy?.troops); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
costs = computeTravelFoodCosts(entityArmy?.troops); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.wheatManager.optimisticResourceUpdate(overrideId, -BigInt(multiplyByPrecision(costs.wheatPayAmount))); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.fishManager.optimisticResourceUpdate(overrideId, -BigInt(multiplyByPrecision(costs.fishPayAmount))); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+339
to
+350
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle potential undefined In the Apply this diff to handle the potential undefined private readonly _optimisticFoodCosts = (overrideId: string, travelType: TravelTypes) => {
const entityArmy = getComponentValue(this.setup.components.Army, this.entity);
+ if (!entityArmy) {
+ // Handle undefined entityArmy, possibly by returning early or setting default costs
+ return;
+ }
let costs = { wheatPayAmount: 0, fishPayAmount: 0 };
if (travelType === TravelTypes.Explore) {
costs = computeExploreFoodCosts(entityArmy.troops);
} else {
costs = computeTravelFoodCosts(entityArmy.troops);
}
// Rest of the code...
}; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private readonly _travelToHex = async (path: HexPosition[], currentArmiesTick: number) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const overrideId = this._optimisticTravelHex( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
path[path.length - 1].col, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -326,10 +374,11 @@ export class ArmyMovementManager { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
directions, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.catch(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Position.removeOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._removeVisualOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._removeNonVisualOverrides(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.then(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.setup.components.Stamina.removeOverride(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._removeNonVisualOverrides(overrideId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Handle Potential Undefined Realm Entity ID
In the constructor,
this.realmEntityId
is assigned usingthis.getRealmEntityId()
, which defaults to0
if theentity_owner_id
is undefined. Ensure that0
is a valid value in this context, or implement error handling for cases whereentity_owner_id
is not found.Consider adding a check and throwing an error or warning if
entity_owner_id
is undefined.