The rev rP19092 triggered a bug. When a global aura target is destroyed, Auras component try to remove its bonus and request StatusBar to regenerate. That part is itself allowed by other things.
Details
- Reviewers
- None
- Trac Tickets
- #4480
(In game "Test":
- take macedonians, build demetrius hero and a boltshooter.
- withtout the patch:
- destroy the Boltshooter: get a warning/error
- unpacked the Boltshooter: you'll get a segfault)
There are different way to prevent that. Basically transforming the error message in StatusBars in an early return should be sufficient (Imo it's too much hidding things).
Input are welcome.
The change in GiveMembersWithValidClass needs to be consider carefully.
If ever that would be commited, it should be after D264.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 1263 Build 1992: Vulcan Build Jenkins Build 1991: arc lint + arc unit
Event Timeline
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/358/ for more details.
That is 'enough' to come back to the previous state but while at it, we can do the good job.
As leper pointed out somewhere, also the Auras itself do things it shouldn't.
There is also something in Pack that I want to put in the table. At last, in StatusBar I don't know if we should let him did bad things (even if not noticeable), checking and throwing error, or just checking.
binaries/data/mods/public/simulation/components/AuraManager.js | ||
---|---|---|
250 | Might be worth commenting why this is useful and/or necessary. |
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/680/ for more details.
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/681/ for more details.
binaries/data/mods/public/simulation/components/AuraManager.js | ||
---|---|---|
250 | Something like "Don't try to apply or remove bonus for units which are destroyed." | |
binaries/data/mods/public/simulation/components/Auras.js | ||
247–250 | units -> entities |
IMO, it should be solved more in the Statusbars and rendering code instead of the Auras. As long as the StatusBars component exists, it should be able to accept all re-rendering requests.
For the regular error message, it should be enough to check the owner != -1 arount line 194 of the statusbars.
But then it all results in a segfault which shouldn't happen (and that's a bug in the C++ part, as no JS code should be able to cause a segfault).
We can indeed just prevent the status bar to regenerate (changing the error in the current diff by an early return)
Or just fix the Pack component and add an early return in the capture bar part.
Or the current diff approach.
Just need some consensus about the best way of doing stuff.
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/877/ for more details.
I think it's now mainly an aura patch. It will be need to rewrite some clean test of Auras component functions.