Changeset View
Standalone View
binaries/data/mods/public/simulation/components/Fogging.js
| Show First 20 Lines • Show All 176 Lines • ▼ Show 20 Lines | |||||
| Fogging.prototype.WasSeen = function(player) | Fogging.prototype.WasSeen = function(player) | ||||
| { | { | ||||
| if (player < 0 || player >= this.seen.length) | if (player < 0 || player >= this.seen.length) | ||||
| return false; | return false; | ||||
| return this.seen[player]; | return this.seen[player]; | ||||
| }; | }; | ||||
| Fogging.prototype.OnDestroy = function(msg) | Fogging.prototype.OnOwnershipChanged = function(msg) | ||||
| { | { | ||||
bb: What is the benefit of merging those two function? appears to me we just add more checks… | |||||
wraitiiAuthorUnsubmitted Done Inline ActionsI think (as in believe) that it's our general practice to use ownershipChanged instead of Destroy.
I could be convinced of not merging them but I don't think it's too bad. wraitii: I think (as in believe) that it's our general practice to use ownershipChanged instead of… | |||||
bbUnsubmitted Not Done Inline Actions
In that case the onDestroy message shouldn't exist anywhere Components with ondestroy but no ownershipchange
Sounds even better if one removes the messages everywhere
False, see messageTypes.h, L226 and further.
There surely are examples where onDestroy could be used instead of ownershipChanged, but there are also examples where it is really the same code, so some duplication argument holds there. bb: > I think (as in believe) that it's our general practice to use ownershipChanged instead of… | |||||
| var cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager); | // Always activate fogging for non-Gaia entities | ||||
| for (var player = 0; player < this.mirages.length; ++player) | if (msg.to > 0) | ||||
| this.Activate(); | |||||
| if (msg.to != -1) | |||||
| return; | |||||
| let cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager); | |||||
| for (let player in this.mirages) | |||||
| { | { | ||||
| if (this.mirages[player] == INVALID_ENTITY) | |||||
| continue; | |||||
| // When this.entity is in the line of sight of the player, its mirage is hidden, rather than destroyed, to save on performance. | |||||
| // All hidden mirages can be destroyed now (they won't be needed again), and other mirages will destroy themselves when they get out of the fog. | |||||
| if (cmpRangeManager.GetLosVisibility(this.mirages[player], player) == "hidden") | if (cmpRangeManager.GetLosVisibility(this.mirages[player], player) == "hidden") | ||||
| { | { | ||||
| Engine.DestroyEntity(this.mirages[player]); | Engine.DestroyEntity(this.mirages[player]); | ||||
| continue; | continue; | ||||
| } | } | ||||
Done Inline ActionsWhat about the following? When this.entity is in the line of sight of the player, its mirage is hidden, rather than destroyed, to save on performance. All hidden mirages can be destroyed now (they won't be needed again), and other mirages will destroy themselves when they get out of the fog. It's easy to confuse the reader by mixing up the "fogged entity" and the visibility status of either entity. Itms: What about the following?
```
When this.entity is in the line of sight of the player, its… | |||||
Done Inline ActionsI'd like to keep this == hidden. Mirages are visible for one turn while they are forwarded to the actual entity (else they "blink"). In this case, it doesn't matter, because the parent entity is destroyed, but for consistency with the rest of the code, I recommend against changing it. Itms: I'd like to keep this `== hidden`. Mirages are `visible` for one turn while they are forwarded… | |||||
| var cmpMirage = Engine.QueryInterface(this.mirages[player], IID_Mirage); | let cmpMirage = Engine.QueryInterface(this.mirages[player], IID_Mirage); | ||||
| if (cmpMirage) | if (cmpMirage) | ||||
| cmpMirage.SetParent(INVALID_ENTITY); | cmpMirage.SetParent(INVALID_ENTITY); | ||||
| } | } | ||||
| }; | }; | ||||
| Fogging.prototype.OnOwnershipChanged = function(msg) | |||||
Done Inline ActionsGood idea! Itms: Good idea! | |||||
| { | |||||
| // Always activate fogging for non-Gaia entities | |||||
| if (msg.to > 0) | |||||
| this.Activate(); | |||||
| }; | |||||
| Fogging.prototype.OnVisibilityChanged = function(msg) | Fogging.prototype.OnVisibilityChanged = function(msg) | ||||
| { | { | ||||
| if (msg.player < 0 || msg.player >= this.mirages.length) | if (msg.player < 0 || msg.player >= this.mirages.length) | ||||
| return; | return; | ||||
| if (msg.newVisibility == VIS_VISIBLE) | if (msg.newVisibility == VIS_VISIBLE) | ||||
| { | { | ||||
| this.miraged[msg.player] = false; | this.miraged[msg.player] = false; | ||||
| this.seen[msg.player] = true; | this.seen[msg.player] = true; | ||||
| } | } | ||||
| if (msg.newVisibility == VIS_FOGGED && this.activated) | if (msg.newVisibility == VIS_FOGGED && this.activated) | ||||
| this.LoadMirage(msg.player); | this.LoadMirage(msg.player); | ||||
| }; | }; | ||||
| Engine.RegisterComponentType(IID_Fogging, "Fogging", Fogging); | Engine.RegisterComponentType(IID_Fogging, "Fogging", Fogging); | ||||
What is the benefit of merging those two function? appears to me we just add more checks, instead of calling the right function