Changeset View
Changeset View
Standalone View
Standalone View
binaries/data/mods/public/simulation/components/ResourceGatherer.js
| Show All 34 Lines | |||||||||||||
| { | { | ||||||||||||
| this.carrying = {}; // { generic type: integer amount currently carried } | this.carrying = {}; // { generic type: integer amount currently carried } | ||||||||||||
| // (Note that this component supports carrying multiple types of resources, | // (Note that this component supports carrying multiple types of resources, | ||||||||||||
| // each with an independent capacity, but the rest of the game currently | // each with an independent capacity, but the rest of the game currently | ||||||||||||
| // ensures and assumes we'll only be carrying one type at once) | // ensures and assumes we'll only be carrying one type at once) | ||||||||||||
| // The last exact type gathered, so we can render appropriate props | // The last exact type gathered, so we can render appropriate props | ||||||||||||
| this.lastCarriedType = undefined; // { generic, specific } | this.lastCarriedType = undefined; // { generic, specific } | ||||||||||||
| this.lastGathered = undefined; | |||||||||||||
Silier: Assignment looks unnecessary, but ok | |||||||||||||
Done Inline ActionsHow would you like me to code it ? it is similar to the line above. mammadori: How would you like me to code it ? it is similar to the line above. | |||||||||||||
Done Inline ActionsYou don't need to assign it at all. Just assign it when using and delete when done with it. (Saves some serialisation data :) ) Freagarach: You don't need to assign it at all. Just assign it when using and delete when done with it. | |||||||||||||
| }; | }; | ||||||||||||
| /** | /** | ||||||||||||
| * Returns data about what resources the unit is currently carrying, | * Returns data about what resources the unit is currently carrying, | ||||||||||||
| * in the form [ {"type":"wood", "amount":7, "max":10} ] | * in the form [ {"type":"wood", "amount":7, "max":10} ] | ||||||||||||
| */ | */ | ||||||||||||
| ResourceGatherer.prototype.GetCarryingStatus = function() | ResourceGatherer.prototype.GetCarryingStatus = function() | ||||||||||||
| { | { | ||||||||||||
| ▲ Show 20 Lines • Show All 47 Lines • ▼ Show 20 Lines | ResourceGatherer.prototype.GetLastCarriedType = function() | ||||||||||||
| return undefined; | return undefined; | ||||||||||||
| }; | }; | ||||||||||||
| ResourceGatherer.prototype.SetLastCarriedType = function(lastCarriedType) | ResourceGatherer.prototype.SetLastCarriedType = function(lastCarriedType) | ||||||||||||
| { | { | ||||||||||||
| this.lastCarriedType = lastCarriedType; | this.lastCarriedType = lastCarriedType; | ||||||||||||
| }; | }; | ||||||||||||
| /** | |||||||||||||
| * Gets the type of the last resource that was gathered. Unlike | |||||||||||||
| * lastCarriedType, this is updated as soon as a unit initiates | |||||||||||||
| * gathering, not when it completes its first full action. | |||||||||||||
| */ | |||||||||||||
| ResourceGatherer.prototype.GetLastGathered = function() | |||||||||||||
| { | |||||||||||||
| if (this.lastGathered) | |||||||||||||
Done Inline ActionsI think check is not needed here, if variable is undefined you can just return it as well with the same result Silier: I think check is not needed here, if variable is undefined you can just return it as well with… | |||||||||||||
| return this.lastGathered; | |||||||||||||
| return undefined; | |||||||||||||
| }; | |||||||||||||
Done Inline ActionsYou only use this within this file and the function does not add any check, so you could just use the member directly below. Freagarach: You only use this within this file and the function does not add any check, so you could just… | |||||||||||||
| ResourceGatherer.prototype.SetLastGathered = function(type) | |||||||||||||
| { | |||||||||||||
| this.lastGathered = type; | |||||||||||||
| }; | |||||||||||||
| // Since this code is very performancecritical and applying technologies quite slow, cache it. | // Since this code is very performancecritical and applying technologies quite slow, cache it. | ||||||||||||
| ResourceGatherer.prototype.RecalculateGatherRatesAndCapacities = function() | ResourceGatherer.prototype.RecalculateGatherRatesAndCapacities = function() | ||||||||||||
| { | { | ||||||||||||
| this.baseSpeed = ApplyValueModificationsToEntity("ResourceGatherer/BaseSpeed", +this.template.BaseSpeed, this.entity); | this.baseSpeed = ApplyValueModificationsToEntity("ResourceGatherer/BaseSpeed", +this.template.BaseSpeed, this.entity); | ||||||||||||
| this.rates = {}; | this.rates = {}; | ||||||||||||
| for (let r in this.template.Rates) | for (let r in this.template.Rates) | ||||||||||||
| { | { | ||||||||||||
| ▲ Show 20 Lines • Show All 216 Lines • ▼ Show 20 Lines | |||||||||||||
| ResourceGatherer.prototype.DropResources = function() | ResourceGatherer.prototype.DropResources = function() | ||||||||||||
| { | { | ||||||||||||
| this.carrying = {}; | this.carrying = {}; | ||||||||||||
| Engine.PostMessage(this.entity, MT_ResourceCarryingChanged, { "to": this.GetCarryingStatus() }); | Engine.PostMessage(this.entity, MT_ResourceCarryingChanged, { "to": this.GetCarryingStatus() }); | ||||||||||||
| }; | }; | ||||||||||||
| // Since we cache gather rates, we need to make sure we update them when tech changes. | // Since we cache gather rates, we need to make sure we update them when tech changes. | ||||||||||||
| // and when our owner change because owners can had different techs. | // and when our owner change because owners can had different techs. | ||||||||||||
Done Inline Actions
Freagarach: | |||||||||||||
Done Inline ActionsCan you (or someone else) come up with a more fitting name? AddToPlayerCounter or something? Freagarach: Can you (or someone else) come up with a more fitting name? `AddToPlayerCounter` or something? | |||||||||||||
Done Inline ActionsCould be UpdatePlayerCount (gatherer is implied I guess) Stan: Could be UpdatePlayerCount (gatherer is implied I guess) | |||||||||||||
| ResourceGatherer.prototype.OnValueModification = function(msg) | ResourceGatherer.prototype.OnValueModification = function(msg) | ||||||||||||
Done Inline Actions
Freagarach: | |||||||||||||
| { | { | ||||||||||||
Done Inline ActionsIs it needed to return something? Freagarach: Is it needed to return something? | |||||||||||||
Done Inline Actions^ Freagarach: ^ | |||||||||||||
Done Inline ActionsNot yet, it was intended as a feedback for UnitAI, I will remove it, it will be re-introducted as necessary. mammadori: Not yet, it was intended as a feedback for UnitAI, I will remove it, it will be re-introducted… | |||||||||||||
| if (msg.component != "ResourceGatherer") | if (msg.component != "ResourceGatherer") | ||||||||||||
Done Inline ActionsQuery this right before it is used. Freagarach: Query this right before it is used. | |||||||||||||
| return; | return; | ||||||||||||
| this.RecalculateGatherRatesAndCapacities(); | this.RecalculateGatherRatesAndCapacities(); | ||||||||||||
Done Inline ActionsNot sure if the warnings actually add anything? Freagarach: Not sure if the warnings actually add anything? | |||||||||||||
Done Inline ActionsThe plan is to remove them as soon as I do not need them, still not clear how it is possible, a dead unit has no player? mammadori: The plan is to remove them as soon as I do not need them, still not clear how it is possible, a… | |||||||||||||
Done Inline ActionsAh, that is what you meant with left debug code in then :) My bad :)
No, a dead entity has a player number of -1. I'm actually nt sure how one would end up with a playerless entity. Freagarach: Ah, that is what you meant with `left debug code in` then :) My bad :)
> a dead unit has no… | |||||||||||||
Done Inline Actionsmove this before cmpPlayer Silier: move this before cmpPlayer | |||||||||||||
| }; | }; | ||||||||||||
| ResourceGatherer.prototype.OnOwnershipChanged = function(msg) | ResourceGatherer.prototype.OnOwnershipChanged = function(msg) | ||||||||||||
| { | { | ||||||||||||
Done Inline ActionsCould just use the member directly? (Since the get doesn't do anything.) Freagarach: Could just use the member directly? (Since the get doesn't do anything.) | |||||||||||||
| if (msg.to == INVALID_PLAYER) | if (msg.to == INVALID_PLAYER) | ||||||||||||
Done Inline Actions
Freagarach: | |||||||||||||
Done Inline Actions
Silier: | |||||||||||||
| return; | return; | ||||||||||||
Done Inline ActionsIs this needed? (Both the comment and the msg.from check.) Freagarach: Is this needed? (Both the comment and the msg.from check.) | |||||||||||||
Done Inline Actionsthe second check is definitely because I'm too chatty on IRC :P can be removed; wraitii: the second check is definitely because I'm too chatty on IRC :P can be removed; | |||||||||||||
| this.RecalculateGatherRatesAndCapacities(); | this.RecalculateGatherRatesAndCapacities(); | ||||||||||||
| }; | }; | ||||||||||||
Done Inline Actions= null is not needed Silier: `= null` is not needed | |||||||||||||
| ResourceGatherer.prototype.OnGlobalInitGame = function(msg) | ResourceGatherer.prototype.OnGlobalInitGame = function(msg) | ||||||||||||
| { | { | ||||||||||||
Done Inline ActionsWhy do you need this? Freagarach: Why do you need this? | |||||||||||||
| this.RecalculateGatherRatesAndCapacities(); | this.RecalculateGatherRatesAndCapacities(); | ||||||||||||
| }; | }; | ||||||||||||
| ResourceGatherer.prototype.OnMultiplierChanged = function(msg) | ResourceGatherer.prototype.OnMultiplierChanged = function(msg) | ||||||||||||
| { | { | ||||||||||||
| let cmpPlayer = QueryOwnerInterface(this.entity, IID_Player); | let cmpPlayer = QueryOwnerInterface(this.entity, IID_Player); | ||||||||||||
Done Inline Actionsremove type variable and use this.lastGathered directly Silier: remove `type` variable and use `this.lastGathered` directly
also move check at the start of the… | |||||||||||||
| if (cmpPlayer && msg.player == cmpPlayer.GetPlayerID()) | if (cmpPlayer && msg.player == cmpPlayer.GetPlayerID()) | ||||||||||||
| this.RecalculateGatherRatesAndCapacities(); | this.RecalculateGatherRatesAndCapacities(); | ||||||||||||
| }; | }; | ||||||||||||
Done Inline ActionsPlease don't use the members of another component directly like this. Freagarach: Please don't use the members of another component directly like this. | |||||||||||||
Done Inline ActionsYou mean the ++ part? How should that be incremented properly? mammadori: You mean the ++ part? How should that be incremented properly?
| |||||||||||||
Done Inline ActionscmpPlayer.AddGatherer(resCode) Freagarach: `cmpPlayer.AddGatherer(resCode)`
(And same comments apply on the decrement function of course.) | |||||||||||||
Done Inline ActionsDon't need the assignment. Freagarach: Don't need the assignment. | |||||||||||||
| Engine.RegisterComponentType(IID_ResourceGatherer, "ResourceGatherer", ResourceGatherer); | Engine.RegisterComponentType(IID_ResourceGatherer, "ResourceGatherer", ResourceGatherer); | ||||||||||||
Done Inline ActionsDon't need to assign that to a variable, right? Freagarach: Don't need to assign that to a variable, right? | |||||||||||||
Done Inline ActionsTo ease reading the following lines. I could use this.lastGathered instead of type if you prefere mammadori: To ease reading the following lines. I could use this.lastGathered instead of type if you… | |||||||||||||
Done Inline ActionsNo strong opinion from me, just asking questions :) This can be just removed if ever I get to refactor treasures. Freagarach: No strong opinion from me, just asking questions :)
This can be just removed if ever I get to… | |||||||||||||
Done Inline Actionsdelete? Freagarach: `delete`? | |||||||||||||
Done Inline ActionsNot sure about the delete though ^^ Freagarach: Not sure about the delete though ^^ | |||||||||||||
Done Inline ActionsWould this be possible? Freagarach: Would this be possible? | |||||||||||||
Done Inline ActionsWhat I mean that if we arrive here without type, something must be wrong, right? Freagarach: What I mean that if we arrive here without type, something must be wrong, right? | |||||||||||||
Done Inline ActionsI'll remove this part too, it was necessary before the using (hopefully correct ) of UnitAI mammadori: I'll remove this part too, it was necessary before the using (hopefully correct ) of UnitAI | |||||||||||||
Wildfire Games · Phabricator
Assignment looks unnecessary, but ok