Changeset View
Standalone View
binaries/data/mods/public/simulation/components/GuiInterface.js
function GuiInterface() {} | function GuiInterface() {} | ||||
GuiInterface.prototype.Schema = | GuiInterface.prototype.Schema = | ||||
"<a:component type='system'/><empty/>"; | "<a:component type='system'/><empty/>"; | ||||
GuiInterface.prototype.Serialize = function() | GuiInterface.prototype.Serialize = function() | ||||
{ | { | ||||
// This component isn't network-synchronised for the biggest part | // This component isn't network-synchronised for the biggest part | ||||
// So most of the attributes shouldn't be serialized | // So most of the attributes shouldn't be serialized | ||||
// Return an object with a small selection of deterministic data | // Return an object with a small selection of deterministic data | ||||
return { | return { | ||||
elexis: We don't want globals in simulation prototypes. Also you're passing the colors from the GUI to… | |||||
Done Inline ActionsI was only passing a bool saying whether to use diplomacy colors or not. But I suppose I could send all the colors instead, if that's a better way of doing it. temple: I was only passing a bool saying whether to use diplomacy colors or not. But I suppose I could… | |||||
"timeNotifications": this.timeNotifications, | "timeNotifications": this.timeNotifications, | ||||
"timeNotificationID": this.timeNotificationID | "timeNotificationID": this.timeNotificationID | ||||
}; | }; | ||||
}; | }; | ||||
GuiInterface.prototype.Deserialize = function(data) | GuiInterface.prototype.Deserialize = function(data) | ||||
{ | { | ||||
this.Init(); | this.Init(); | ||||
▲ Show 20 Lines • Show All 67 Lines • ▼ Show 20 Lines | for (let j = 0; j < numPlayers; ++j) | ||||
mutualAllies[j] = cmpPlayer.IsMutualAlly(j); | mutualAllies[j] = cmpPlayer.IsMutualAlly(j); | ||||
neutrals[j] = cmpPlayer.IsNeutral(j); | neutrals[j] = cmpPlayer.IsNeutral(j); | ||||
enemies[j] = cmpPlayer.IsEnemy(j); | enemies[j] = cmpPlayer.IsEnemy(j); | ||||
} | } | ||||
ret.players.push({ | ret.players.push({ | ||||
"name": cmpPlayer.GetName(), | "name": cmpPlayer.GetName(), | ||||
"civ": cmpPlayer.GetCiv(), | "civ": cmpPlayer.GetCiv(), | ||||
"color": cmpPlayer.GetColor(), | "color": cmpPlayer.GetColor(), | ||||
Not Done Inline ActionsThis should never return the diplo color, right? elexis: This should never return the diplo color, right? | |||||
Not Done Inline ActionsDepends if you want to rewrite the GUI or not. Session grabs the player color from here and uses that everywhere in the GUI, so it should be the same as the displayed color. (In session I set g_Players[i].color directly rather than doing onSimulationUpdate because I don't need to update all this other data plus I need to calculate the array of diplomacy colors anyway.) temple: Depends if you want to rewrite the GUI or not. Session grabs the player color from here and… | |||||
Not Done Inline Actions(Sorry, it wasn't onSimulationUpdate but updatePlayerData that refreshes the colors.) temple: (Sorry, it wasn't onSimulationUpdate but updatePlayerData that refreshes the colors.) | |||||
"controlsAll": cmpPlayer.CanControlAllUnits(), | "controlsAll": cmpPlayer.CanControlAllUnits(), | ||||
"popCount": cmpPlayer.GetPopulationCount(), | "popCount": cmpPlayer.GetPopulationCount(), | ||||
"popLimit": cmpPlayer.GetPopulationLimit(), | "popLimit": cmpPlayer.GetPopulationLimit(), | ||||
"popMax": cmpPlayer.GetMaxPopulation(), | "popMax": cmpPlayer.GetMaxPopulation(), | ||||
"panelEntities": cmpPlayer.GetPanelEntities(), | "panelEntities": cmpPlayer.GetPanelEntities(), | ||||
"resourceCounts": cmpPlayer.GetResourceCounts(), | "resourceCounts": cmpPlayer.GetResourceCounts(), | ||||
"trainingBlocked": cmpPlayer.IsTrainingBlocked(), | "trainingBlocked": cmpPlayer.IsTrainingBlocked(), | ||||
"state": cmpPlayer.GetState(), | "state": cmpPlayer.GetState(), | ||||
▲ Show 20 Lines • Show All 667 Lines • ▼ Show 20 Lines | if (!cmpBuilder) | ||||
continue; | continue; | ||||
for (let building of cmpBuilder.GetEntitiesList()) | for (let building of cmpBuilder.GetEntitiesList()) | ||||
if (buildableEnts.indexOf(building) == -1) | if (buildableEnts.indexOf(building) == -1) | ||||
buildableEnts.push(building); | buildableEnts.push(building); | ||||
} | } | ||||
return buildableEnts; | return buildableEnts; | ||||
}; | }; | ||||
GuiInterface.prototype.UpdateDisplayedPlayerColors = function(player, data) | |||||
{ | |||||
Done Inline Actions(can be shortened by one line) elexis: (can be shortened by one line) | |||||
let cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager); | |||||
let numPlayers = Engine.QueryInterface(SYSTEM_ENTITY, IID_PlayerManager).GetNumPlayers(); | |||||
for (let i = 1; i < numPlayers; ++i) | |||||
{ | |||||
let cmpPlayer = QueryPlayerIDInterface(i, IID_Player); | |||||
Not Done Inline Actionscould pass data[i] elexis: could pass `data[i]` | |||||
Not Done Inline ActionsSetColor is done this way so I was being consistent. temple: SetColor is done this way so I was being consistent. | |||||
if (!cmpPlayer) | |||||
continue; | |||||
cmpPlayer.SetDisplayDiplomacyColor(data.displayDiplomacyColors); | |||||
if (data.displayDiplomacyColors) | |||||
cmpPlayer.SetDiplomacyColor(data.displayedPlayerColors[i]); | |||||
Done Inline Actionsextra line temple: extra line | |||||
for (let ent of cmpRangeManager.GetEntitiesByPlayer(i)) | |||||
Not Done Inline ActionsReading this function, one might still think it's cleaner to pass the diplomacy color as an argument here. In case the diplomacyColor display is disabled, the GUIInterface would have to either pass the playerColor to the simulation components or the simulation components conditionally get their color from two different components. Not sure if avoiding the hardcoded simulation component names by sending an MT_NonSyncPlayerColor simulation message would be worth the invitation to add a non-synced code. Also no clue about the performance overhead of the messages. elexis: Reading this function, one might still think it's cleaner to pass the diplomacy color as an… | |||||
{ | |||||
for (let iid of [IID_Minimap, IID_RangeOverlayRenderer, IID_RallyPointRenderer]) | |||||
{ | |||||
let cmp = Engine.QueryInterface(ent, iid); | |||||
if (cmp) | |||||
cmp.UpdateColor(); | |||||
} | |||||
if (data.showAllStatusBars && (i == player || player == INVALID_PLAYER)) | |||||
elexisUnsubmitted Done Inline ActionsINVALID_PLAYER is the observer playerID here? I suspect we want an own constant for that, (I've left it at -1 in some observer places iirc since not sure) The condition is independent of the entityID, so it can be computed once outside of the loop. And if we do that already, we might just as well construct the IID array which is also independent of the entityID outside of the loop too. And then we can add an UpdateColor funciton to cmpStatusBars that calls RegenerateSprites (and possibly more in the future) to unify that part under the IID loop too. elexis: INVALID_PLAYER is the observer playerID here? I suspect we want an own constant for that, (I've… | |||||
{ | |||||
let cmpStatusBars = Engine.QueryInterface(ent, IID_StatusBars); | |||||
if (cmpStatusBars) | |||||
cmpStatusBars.RegenerateSprites(); | |||||
} | |||||
Not Done Inline Actions(Sending a simulation event is indeed something that nicely avoids having to specify to hardcode the components that need to be informed. elexis: (Sending a simulation event is indeed something that nicely avoids having to specify to… | |||||
} | |||||
Not Done Inline ActionsMaybe that becomes more readable (some irrelevant nanoseconds more performant) ìf we don't pass an IID array but call updateEntityColors for one IID. The entities wouldn't be updated one by one anymore. Anyway, very good code as is. elexis: Maybe that becomes more readable (some irrelevant nanoseconds more performant) ìf we don't pass… | |||||
} | |||||
for (let ent of data.selected) | |||||
{ | |||||
let cmpSelectable = Engine.QueryInterface(ent, IID_Selectable); | |||||
Done Inline ActionsLooks good! packed, no redundancy. :-) for iiids... for entityIDs... cmpUpdate }; elexis: Looks good! packed, no redundancy. :-)
The second IID loop could be the outer loop, then the… | |||||
Done Inline Actions(If it becomes a function, the IIDs can remain the inner loop too) elexis: (If it becomes a function, the IIDs can remain the inner loop too) | |||||
if (cmpSelectable) | |||||
cmpSelectable.UpdateColor(); | |||||
let cmpStatusBars = Engine.QueryInterface(ent, IID_StatusBars); | |||||
if (cmpStatusBars) | |||||
cmpStatusBars.RegenerateSprites(); | |||||
Done Inline ActionsWe should replace that -1 with a constant some day. elexis: We should replace that -1 with a constant some day. | |||||
elexisUnsubmitted Not Done Inline ActionsRegenerateSprites is run twice for the same selected entity owned by a player if showAllStatusBars is true. elexis: `RegenerateSprites` is run twice for the same selected entity owned by a player if… | |||||
templeAuthorUnsubmitted Not Done Inline ActionsWe can select at most 200 units, so there's a cap on how bad this part can be (~20ms, maybe). temple: We can select at most 200 units, so there's a cap on how bad this part can be (~20ms, maybe). | |||||
elexisUnsubmitted Not Done Inline ActionsThat 200 might become 300 eventually, not set in stone, but realistically capped yes. elexis: That 200 might become 300 eventually, not set in stone, but realistically capped yes.
Yeah not… | |||||
} | |||||
Engine.QueryInterface(SYSTEM_ENTITY, IID_TerritoryManager).UpdateColors(); | |||||
}; | |||||
Done Inline ActionsThe entire block could be minimized by mandating the updated functions to have an UpdateDisplayedPlayerColor function and calling that in a loop over the component interface IDs. elexis: The entire block could be minimized by mandating the updated functions to have an… | |||||
GuiInterface.prototype.SetSelectionHighlight = function(player, cmd) | GuiInterface.prototype.SetSelectionHighlight = function(player, cmd) | ||||
Done Inline ActionsGaia entities are not update (aka initialized?) anymore elexis: Gaia entities are not update (aka initialized?) anymore | |||||
Done Inline ActionsI remember leper saying comments for getter and setter were useless . has the policy changed ? Stan: I remember leper saying comments for getter and setter were useless . has the policy changed ? | |||||
Not Done Inline ActionsKeeping MT_PlayerColorChanged while adding an MT_DisplayedPlayerColorChanged or MT_AsyncDisplayedPlayerColorChanged changed event might be sufficient to stop simulation authors from executing serialization relevant code in the processing of that event and would allow removing the hardcoded component names in the GUIInterface however. We also remember the idea to push the color from the GUI Interface to the components Update function or message (this way it wouldn't have to be saved in the Player component and the serialization function wouldn't have to be changed). But the current code is sufficient and good (and has the advantage of not having it possible for people to misuse the MT_..) elexis: Keeping `MT_PlayerColorChanged` while adding an `MT_DisplayedPlayerColorChanged` or… | |||||
{ | { | ||||
let playerColors = {}; // cache of owner -> color map | let playerColors = {}; // cache of owner -> color map | ||||
for (let ent of cmd.entities) | for (let ent of cmd.entities) | ||||
{ | { | ||||
let cmpSelectable = Engine.QueryInterface(ent, IID_Selectable); | let cmpSelectable = Engine.QueryInterface(ent, IID_Selectable); | ||||
if (!cmpSelectable) | if (!cmpSelectable) | ||||
continue; | continue; | ||||
// Find the entity's owner's color: | // Find the entity's owner's color: | ||||
let owner = INVALID_PLAYER; | let owner = INVALID_PLAYER; | ||||
let cmpOwnership = Engine.QueryInterface(ent, IID_Ownership); | let cmpOwnership = Engine.QueryInterface(ent, IID_Ownership); | ||||
if (cmpOwnership) | if (cmpOwnership) | ||||
owner = cmpOwnership.GetOwner(); | owner = cmpOwnership.GetOwner(); | ||||
let color = playerColors[owner]; | let color = playerColors[owner]; | ||||
if (!color) | if (!color) | ||||
{ | { | ||||
color = { "r":1, "g":1, "b":1 }; | color = { "r":1, "g":1, "b":1 }; | ||||
let cmpPlayer = QueryPlayerIDInterface(owner); | let cmpPlayer = QueryPlayerIDInterface(owner); | ||||
if (cmpPlayer) | if (cmpPlayer) | ||||
color = cmpPlayer.GetColor(); | color = cmpPlayer.GetDisplayedColor(); | ||||
Not Done Inline Actions(Using a ternary or else-statement would only assign a value if its used) elexis: (Using a ternary or else-statement would only assign a value if its used) | |||||
playerColors[owner] = color; | playerColors[owner] = color; | ||||
} | } | ||||
cmpSelectable.SetSelectionHighlight({ "r": color.r, "g": color.g, "b": color.b, "a": cmd.alpha }, cmd.selected); | cmpSelectable.SetSelectionHighlight({ "r": color.r, "g": color.g, "b": color.b, "a": cmd.alpha }, cmd.selected); | ||||
let cmpRangeOverlayManager = Engine.QueryInterface(ent, IID_RangeOverlayManager); | let cmpRangeOverlayManager = Engine.QueryInterface(ent, IID_RangeOverlayManager); | ||||
if (!cmpRangeOverlayManager || player != owner && player != INVALID_PLAYER) | if (!cmpRangeOverlayManager || player != owner && player != INVALID_PLAYER) | ||||
continue; | continue; | ||||
▲ Show 20 Lines • Show All 893 Lines • ▼ Show 20 Lines | |||||
* @param idleclasses Array of class names to include. | * @param idleclasses Array of class names to include. | ||||
* @param excludeUnits Array of units to exclude. | * @param excludeUnits Array of units to exclude. | ||||
* | * | ||||
* Returns an object with the following fields: | * Returns an object with the following fields: | ||||
* - idle - true if the unit is considered idle by the filter, false otherwise. | * - idle - true if the unit is considered idle by the filter, false otherwise. | ||||
* - bucket - if idle, set to the index of the first matching idle class, undefined otherwise. | * - bucket - if idle, set to the index of the first matching idle class, undefined otherwise. | ||||
*/ | */ | ||||
GuiInterface.prototype.IdleUnitFilter = function(unit, idleClasses, excludeUnits) | GuiInterface.prototype.IdleUnitFilter = function(unit, idleClasses, excludeUnits) | ||||
{ | { | ||||
Done Inline ActionscmpPlayer2.SetDiplomacyColor(...g_DiplomacyColors[teamColors[team] || i]);? elexis: `cmpPlayer2.SetDiplomacyColor(...g_DiplomacyColors[teamColors[team] || i]);`? | |||||
Done Inline ActionsNeat. temple: Neat. | |||||
let cmpUnitAI = Engine.QueryInterface(unit, IID_UnitAI); | let cmpUnitAI = Engine.QueryInterface(unit, IID_UnitAI); | ||||
if (!cmpUnitAI || !cmpUnitAI.IsIdle() || cmpUnitAI.IsGarrisoned()) | if (!cmpUnitAI || !cmpUnitAI.IsIdle() || cmpUnitAI.IsGarrisoned()) | ||||
return { "idle": false }; | return { "idle": false }; | ||||
let cmpIdentity = Engine.QueryInterface(unit, IID_Identity); | let cmpIdentity = Engine.QueryInterface(unit, IID_Identity); | ||||
if(!cmpIdentity) | if(!cmpIdentity) | ||||
return { "idle": false }; | return { "idle": false }; | ||||
let bucket = idleClasses.findIndex(elem => MatchesClassList(cmpIdentity.GetClassesList(), elem)); | let bucket = idleClasses.findIndex(elem => MatchesClassList(cmpIdentity.GetClassesList(), elem)); | ||||
if (bucket == -1 || excludeUnits.indexOf(unit) > -1) | if (bucket == -1 || excludeUnits.indexOf(unit) > -1) | ||||
return { "idle": false }; | return { "idle": false }; | ||||
return { "idle": true, "bucket": bucket }; | return { "idle": true, "bucket": bucket }; | ||||
}; | }; | ||||
GuiInterface.prototype.GetTradingRouteGain = function(player, data) | GuiInterface.prototype.GetTradingRouteGain = function(player, data) | ||||
{ | { | ||||
if (!data.firstMarket || !data.secondMarket) | if (!data.firstMarket || !data.secondMarket) | ||||
Done Inline ActionsWe need i at most once. Are those good property names? Color changed from player X to player X? elexis: We need `i` at most once. Are those good property names? Color changed from player X to player… | |||||
Done Inline ActionsI did it this way so I wouldn't have to copy/paste cpp code since MT_OwnershipChanged was doing the same thing in Minimap and Selectable, and almost the same thing in TerritoryManager. Maybe we could remove the MT_OwnershipChanged case in Minimap and Selectable and instead use MT_ColorChanged, and instead of from/to send the r/g/b values. It seems that MT_OwnershipChanged is only posted once, in Player.js, so it'd be easy enough to also post an MT_ColorChanged message there. TerritoryManager is still awkward though. temple: I did it this way so I wouldn't have to copy/paste cpp code since MT_OwnershipChanged was doing… | |||||
return null; | return null; | ||||
return CalculateTraderGain(data.firstMarket, data.secondMarket, data.template); | return CalculateTraderGain(data.firstMarket, data.secondMarket, data.template); | ||||
}; | }; | ||||
GuiInterface.prototype.GetTradingDetails = function(player, data) | GuiInterface.prototype.GetTradingDetails = function(player, data) | ||||
{ | { | ||||
let cmpEntityTrader = Engine.QueryInterface(data.trader, IID_Trader); | let cmpEntityTrader = Engine.QueryInterface(data.trader, IID_Trader); | ||||
▲ Show 20 Lines • Show All 166 Lines • ▼ Show 20 Lines | let exposedFunctions = { | ||||
"GetAvailableFormations": 1, | "GetAvailableFormations": 1, | ||||
"GetFormationRequirements": 1, | "GetFormationRequirements": 1, | ||||
"CanMoveEntsIntoFormation": 1, | "CanMoveEntsIntoFormation": 1, | ||||
"IsFormationSelected": 1, | "IsFormationSelected": 1, | ||||
"GetFormationInfoFromTemplate": 1, | "GetFormationInfoFromTemplate": 1, | ||||
"IsStanceSelected": 1, | "IsStanceSelected": 1, | ||||
"UpdateDisplayedPlayerColors": 1, | |||||
"SetSelectionHighlight": 1, | "SetSelectionHighlight": 1, | ||||
"GetAllBuildableEntities": 1, | "GetAllBuildableEntities": 1, | ||||
"SetStatusBars": 1, | "SetStatusBars": 1, | ||||
"GetPlayerEntities": 1, | "GetPlayerEntities": 1, | ||||
"GetNonGaiaEntities": 1, | "GetNonGaiaEntities": 1, | ||||
"DisplayRallyPoint": 1, | "DisplayRallyPoint": 1, | ||||
"AddTargetMarker": 1, | "AddTargetMarker": 1, | ||||
"SetBuildingPlacementPreview": 1, | "SetBuildingPlacementPreview": 1, | ||||
Show All 34 Lines |
We don't want globals in simulation prototypes. Also you're passing the colors from the GUI to the GUI interface as an argument and then don't use that argument. So nuke this global and use the argument.