Resource gain on death

[basxto]

https://www.hiveworkshop.com/threads/simple-tower-defense-v3.80868/ tends to crash with:

Exception in thread "LWJGL Application" java.lang.NullPointerException: Cannot read field "location" because "renderPeer" is null
	at com.etheller.warsmash.viewer5.handlers.w3x.War3MapViewer$MapLoader$1.spawnTextTag(War3MapViewer.java:3239)
	at com.etheller.warsmash.viewer5.handlers.w3x.simulation.CSimulation.spawnTextTag(CSimulation.java:773)
	at com.etheller.warsmash.viewer5.handlers.w3x.simulation.CSimulation.unitGainResourceEvent(CSimulation.java:761)
	at com.etheller.warsmash.viewer5.handlers.w3x.simulation.CUnit.kill(CUnit.java:3056)
	at com.etheller.warsmash.viewer5.handlers.w3x.simulation.CUnit.damage(CUnit.java:2939)
	at com.etheller.warsmash.viewer5.handlers.w3x.simulation.combat.attacks.CUnitAttackMissile.doDamage(CUnitAttackMissile.java:102)
	at com.etheller.warsmash.viewer5.handlers.w3x.simulation.combat.projectile.CAttackProjectileMissile.onHitTarget(CAttackProjectileMissile.java:27)
	at com.etheller.warsmash.viewer5.handlers.w3x.simulation.combat.projectile.CProjectile.update(CProjectile.java:60)
	at com.etheller.warsmash.viewer5.handlers.w3x.simulation.CSimulation.update(CSimulation.java:534)
	at com.etheller.warsmash.viewer5.handlers.w3x.War3MapViewer.update(War3MapViewer.java:1185)
	at com.etheller.warsmash.viewer5.ModelViewer.updateAndRender(ModelViewer.java:275)
	at com.etheller.warsmash.WarsmashGdxMapScreen.render(WarsmashGdxMapScreen.java:289)
	at com.badlogic.gdx.Game.render(Game.java:48)
	at com.badlogic.gdx.backends.lwjgl.LwjglApplication.mainLoop(LwjglApplication.java:259)
	at com.badlogic.gdx.backends.lwjgl.LwjglApplication$1.run(LwjglApplication.java:138)

At first sight it looks like the killed unit gets removed (or garbage collected) before the +1 text for resource gain can be written, but it needs to know the unit position to know where to write it.

#75 now contains a possible fix for this

[Retera]

So I would not normally assume that a dead unit would cease to have a render peer, suggesting that the issue here might be coming from a custom map with a trigger to remove units in response to their death which may have been firing prior to the spawning of this gold text tag.

In your changes #75 you have allowed for the text to to spawn at the XY of a removed unit, but the Z of the removed unit would still be lost based on my read of the code. Most likely this means that on the original 2002 game a flying unit in your custom map, if it is killed, will show the +1 text tag in the air at the location of the flying unit, but now on your suggested pull request will remove the unit from the userland trigger script, then spawn +1 text tag on the ground at the feet beneath where the flying unit had been -- because his height is already lost.

Is it possibly the case instead that the better fix is to spawn the text tag (via unitGainResourceEvent) by calling that method prior to the execution of any userland trigger scripts in response to the death of the unit?

For pull request review, it might also be helpful to link which map file you were using to test.

[basxto]

For pull request review, it might also be helpful to link which map file you were using to test.

I linked it at the top? https://www.hiveworkshop.com/threads/simple-tower-defense-v3.80868/

That’s the only map I’m playing. I didn’t manage to get any other scripted map working.

Is it possibly the case instead that the better fix is to spawn the text tag (via unitGainResourceEvent) by calling that method prior to the execution of any userland trigger scripts in response to the death of the unit?

Sure. The map has a lot of visual issues and my main goal was to not crash while playing it. I just call the method that already had more elaborate checks, since they otherwise seemed to do the same thing.

--

That crash only occured later in the map. I think it was the flying wave, where the ultra creep has black textures in warsmash, but I could be wrong. That it only occured rarely could mean that it comes from the crystal gain (only ultra creeps) and not gold gain (all creeps).

[Glasislundr]

Based on Retera's comment and my baseless guess, it's probably related to this part of the CUnit's update method:

					if (gameTurnTick > (this.deathTurnTick
							+ (int) (this.unitType.getDeathTime() / WarsmashConstants.SIMULATION_STEP_TIME))) {
						this.corpse = true;
						if (!isRaisable()) {
							this.boneCorpse = true;
							// start final phase immediately for "cant raise" case
						}
						if (!this.unitType.isHero()) {
							if (!isDecays()) {
								// if we dont raise AND dont decay, then now that death anim is over
								// we just delete the unit
								return true;
							}
						} else {
							game.heroDeathEvent(this);
						}
						this.deathTurnTick = gameTurnTick;
					}

Notably, when the CUnit's update method returns "true", the CSimulation immediately deletes the RenderUnit connected to it (which holds the location data). Flying units don't decay, and if they have a 0 "DeathTime" then the RenderUnit would be deleted exactly as the unit dies.