Opened 10 years ago
Closed 10 years ago
#2285 closed defect (fixed)
Damage helper holds simulation aware data in static function
Reported by: | sanderd17 | Owned by: | Josh |
---|---|---|---|
Priority: | Release Blocker | Milestone: | Alpha 15 |
Component: | UI & Simulation | Keywords: | |
Cc: | Josh | Patch: |
Description
The Damage helper script creates a new entity on the first missile fired and missed
Damage.dummyTargetEntity = Engine.AddEntity('special/dummy');
As the script is static, the dummyTargetEntity is always remembered, even if the game is terminated (without the engine closed). You can see this in Atlas:
- Start on an empty map
- place at least two enemy archers on the map, close to each other
- start the simulation and let them fire until at least one arrow missed target
- reset the simulation
- Add a new entity, somewhere else on the map
- When the simulation is started, the unit jumps to all positions of missed arrows, as it got the ID of the dummyTarget, and the id of the dummyTarget wasn't deleted.
Depending on the number of entities in previous games, it can even cause OOS problems if people didn't restart the entire game before starting a new game.
This should be solved properly, by adding a method to RangeManager that allows a range query around a position instead of just around an entity.
Attachments (1)
Change History (11)
comment:1 by , 10 years ago
Priority: | Release Blocker → Should Have |
---|
comment:3 by , 10 years ago
Priority: | Should Have → Release Blocker |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
There was a report about an OOS problem after rejoining the game on IRC last night: http://irclogs.wildfiregames.com/2013-11-30-QuakeNet-%230ad.log (around 23:12)
It's quite clear that this problem has the same source. One player still has the Damage script running, with an entity ID attached to it, while that ID isn't serialized, so if the second player joins again, a different ID is used. Which causes different IDs for all following entities.
comment:4 by , 10 years ago
Hmm, well I'll try to look at adding another method to the range manager again. Last time I got rather lost, but I know C++ far better now than I did a few months ago so I should be able to figure it out.
comment:5 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | reopened → new |
by , 10 years ago
Attachment: | damage.diff added |
---|
comment:6 by , 10 years ago
The attached diff should work, but I have no idea if the style is good enough.
comment:7 by , 10 years ago
Looks good to me, except you're losing some of the validation in PerformQuery which should be added back.
if (!cmpSourcePosition || !cmpSourcePosition->IsInWorld()) return;
comment:8 by , 10 years ago
Well, now the method that calls PerformQuery is supposed to give the position. And those methods also do the in-world checking (I can't check if the entity is in world if the position is already given).
All those methods already needed the entity position for themselves, as they needed to sort on that position after running the PerformQuery. I just made the query for the position a bit earlier.
So it should be fine.
comment:9 by , 10 years ago
Alright then, after another quick review, I would just recommend committing it.
In r14254 I triaged the problem, but that also entailed removing any sort of entity caching so it's now less efficient then before. Long-term solution would be to add a new RangeManager method.