Page MenuHomeWildfire Games
Differential D1707 Diff 7514 binaries/data/mods/public/simulation/components/Attack.js

Changeset View

Standalone View

View Options

binaries/data/mods/public/simulation/components/Attack.js

Context not available.
let template = this.template[type]; let template = this.template[type];
if (!template) if (!template)
template = this.template[type.split(".")[0]].Splash; template = this.template[type.split(".")[0]].Splash;
if (template.Bonuses) {
StanUnsubmitted
Done
Inline Actions

{ on new line.

Stan: { on new line.
for (let bonus in template.Bonuses)
bonus.multiplier = ApplyValueModificationsToEntity("Attack/" + type + "/Bonuses/" + bonus + "/Multiplier", +template.Bonuses[bonus].Multiplier, this.entity);
StanUnsubmitted
Done
Inline Actions

You forgot that change :)

Stan: You forgot that change :)
SilierUnsubmitted
Done
Inline Actions

let bonus creates local variable which is changed and forgot after every loop iteration. You are not changing nothing in template.Bonuses which you are returning later

Silier: let bonus creates local variable which is changed and forgot after every loop iteration. You…
lyvUnsubmitted
Done
Inline Actions

let bonus creates local variable which is changed and forgot after every loop iteration.

Not necessarily.

var a = [{"a": 4, "b": 7}, {"a": 5, "b": 8}
for (let obj of a) obj.b += 10;
warn(uneval(a));

Objects are passed by reference. This would have been correct if you were not iterating the keys. If you print out bonus in the loop, you would only see the key names in the first object. Not the object the key corresponds to.
I have no clue about the semantics of this change. (Is this needed or not?)

lyv: > let bonus creates local variable which is changed and forgot after every loop iteration. Not…
StanUnsubmitted
Done
Inline Actions

Change is from another diff of mine :)
It allows bonus modifications from tech.

Stan: Change is from another diff of mine :) It allows bonus modifications from tech.
lyvUnsubmitted
Done
Inline Actions

Ah, I see.

lyv: Ah, I see.
StanUnsubmitted
Done
Inline Actions

Actually I was wrong, this is not from my patch, but it is indeed the reason why my patch and this one are not compatible. Since the multiplier do not get applied correctly here, the tooltip is always wrong.
This should do the trick I think.

if (template.Bonuses) 
{
	for (let bonusKey in template.Bonuses)
	{
		let bonus = template.Bonuses[bonusKey];
		bonus.Multiplier = ApplyValueModificationsToEntity("Attack/" + type + "/Bonuses/" + bonusKey + "/Multiplier", +bonus.Multiplier, this.entity);
	}

}
Stan: Actually I was wrong, this is not from my patch, but it is indeed the reason why my patch and…
}
return template.Bonuses || null; return template.Bonuses || null;
}; };
Context not available.
elexisUnsubmitted
Done
Inline Actions

The clone call could be avoided by constructing the output object in place (would also look a bit cleaner).
Secondly this is called often (each attack, each guiinterface call) but changed rarely, perhaps its better to cache it (even if it means one more variable to be stored memory - and even serialized unless explicitly removed from the Serialize function and recreated in the Deserialize function)

elexis: The `clone` call could be avoided by constructing the output object in place (would also look a…
elexisUnsubmitted
Done
Inline Actions

(offtopic, but above line sounds like a hack)

elexis: (offtopic, but above line sounds like a hack)
StanUnsubmitted
Done
Inline Actions

Well with the caching now it's duplicated. Enjoy.

Stan: Well with the caching now it's duplicated. Enjoy.
Wildfire Games · Phabricator