Opened 7 years ago
Closed 4 years ago
#4420 closed enhancement (fixed)
[PATCH] UnitMotion rewrite
Reported by: | wraitii | Owned by: | wraitii |
---|---|---|---|
Priority: | Must Have | Milestone: | Alpha 24 |
Component: | Simulation | Keywords: | patch |
Cc: | Patch: |
Description (last modified by )
UnitMotion has always been quite annoying to change, and full of a variety of complicated bugs. Its ties with unitAI are also quite annoying. I have rewritten it. "Big" changes: -Dumped the m_State/m_PathState variables, and generally streamlined the unitMotion logic, implemented an easier way to wait some turns when paths fail and react accordingly (should reduce units getting stuck and the number of potential bugs in general). -Let UnitMotion handle the move animations instead of hacking it with unitAI. This means that ultimately we could have "walk-attack", "run-gather" animations. But for now we don't really support that. -Changed speed to just walk speed, and use a ratio to change it instead of an absolute value. Ultimately, I plan te delete run speeds, but haven't gotten around to it yet. In some finer points, I dump the "longPath"/"shortPath" split to just be a simple "Path", so that if we remove the short pathfinder someday that will be easier, and it just makes the whole thing nicer. I also have a test map, available on my github branches: -Working branch: https://github.com/wraitii/0ad/tree/UnitMotionRewrite -"Cleaned up" branch: https://github.com/wraitii/0ad/tree/UMRewrite_Clean Cleaned up branch might be slightly easier to review. Attached unified diff (sans map) below.
Attachments (1)
Change History (10)
by , 7 years ago
Attachment: | unitMotionRewrite.patch added |
---|
comment:1 by , 7 years ago
Note that this isn't a complete rewrite, but PathResult and Move have been mostly rewritten.
TODOs: -Make unitmotionFlying work again (forgot we use that for birds). -make serialization work again -Change unitAI IsIdle maybe to call IsTryingToMove? -Give the AI scripts information maybe? -Update templates for my running changes -Probably scrap the whole fleeing logic, it's stupid -Make visual actor nicer, handle "stops" mid-animations -Do some of the TODOs in the files. -Improve test map.
Edit: oh yeah and formation stuff. Completely broken now.
edit2: also to clarify what I want: the above patch should work as long as you don't use formations, so I'm looking for a review and some testing if possible, obviously not about the TODOs above though. I will post a separate patch to go on top of this one with those TODOs, to make reviewing possibly slightly easier.
edit3: oh yeah also I have a weird bug where attack animations seem to go super slowly for some reason. Not sure sure it's cause by these changes though.
comment:3 by , 7 years ago
It's a big work. I will try to (understand the part I didn't know), review and test. But my sole input will not be sufficient. It would be nice if we put energy on that now, then we will have the whole release process to investigate unexpected bugs.
comment:4 by , 7 years ago
Description: | modified (diff) |
---|---|
Keywords: | review removed |
review is taking place on Phabricator: https://code.wildfiregames.com/D13
comment:5 by , 7 years ago
Milestone: | Alpha 22 → Work In Progress |
---|
Moving to the Work In Progress milestone, since there is a patch asking for feedback, but since it is not strictly bound to a specific release.
comment:6 by , 5 years ago
Component: | UI & Simulation → Simulation |
---|
Move tickets to Simulation
as UI & Simulation
got some sub components.
comment:7 by , 5 years ago
Description: | modified (diff) |
---|---|
Owner: | set to |
Re-assigning as I'm currently committing the v2 of this rewrite. Here's a list of reported issues, along with status:
Angen reports that fleeing units don't get chased efficiently.fixed, fleeing itself needs fixing.There was a report of an issue with attacking units but I haven't been able to understand what exactly.probably fixed by rP22313 followups.
- gameboy reports that armies can't reach his warships. As that sounds more game-breaking, I'll look into it right away. ( edit: https://code.wildfiregames.com/D1963)
- Freagarach reported a number of issues there: https://code.wildfiregames.com/rP22345#33870
- Wow reported issues there: https://wildfiregames.com/forum/index.php?/topic/26202-changeset22355/
- except for self-noted below, this says that units don't always face the correct point - stopMoving() appears to be called, so perhaps the correct setting isn't set - perhaps the behaviour of facing towards the final point is wrong.
- actually, it seems that sometimes we just need to face explicitly perhaps.
- except for self-noted below, this says that units don't always face the correct point - stopMoving() appears to be called, so perhaps the correct setting isn't set - perhaps the behaviour of facing towards the final point is wrong.
Self noted:
- animal roaming looks weird - I believe this might be a unit motion issue somewhat - maybe just weirdness resulting in broken behaviour - check after upstream has landed
- Fleeing/Chasing:
- Forgot to change CHASING to also use MoveToAttackingRange, which was why it looked weird. I think fleeing is fine with that fixed.
- UnitMotion does not send a "MoveCompleted" update when we reach the end of a path to a waypoint.
This explains the "animals won't leave foundations twice" bug, and the "units sometimes keep fleeing forever" bug. Will be fixed upstream in D1899.- Actually incorrect, the cause for these two bugs were that we didn't support infinite max range, and trickiness with minimum range calculations.
Perhaps related to https://wildfiregames.com/forum/index.php?/topic/26195-my-army-cant-log-on-to-the-warship/&tab=comments#comment-377828
- as reported by gameboy, return-resources missed an interface which made it sometimes break - was kinda dependent on motion since the order hid that bug sometimes. https://wildfiregames.com/forum/index.php?/topic/26195-my-army-cant-log-on-to-the-warship/&tab=comments#comment-377828 Fixed by https://code.wildfiregames.com/D1975
- minohaka reports that units sometimes get stuck in GATHER.APPROACHING when the targeted entity no longer exists. Edit: fixed by D1979
- krinkle shows in #5454 that animals sometimes don't move away from foundations - need to figure that out. Edit: I believe that's the same issue that's fixed by D1969 . Edit: also reported by wow, again I think it's the same issue.
- another report by minohaka that chained orders fail: https://wildfiregames.com/forum/index.php?/topic/25155-testing-pathfinder-update/page/4/&tab=comments#comment-378025 . Probably just units being too resilient.
- as reported by Angen, range checks in APPROACHING are a bit too gloomy and should not happen in case of errors - we should handle this gracefully. See D1992.
comment:8 by , 5 years ago
Milestone: | Work In Progress → Alpha 24 |
---|
comment:9 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Issues noted above were fixed. Remaining issues will have gotten specific tickets in the meantime.
Basic version - working but needs refinements.