Opened 8 years ago
Closed 6 years ago
#4268 closed defect (fixed)
[PATCH] Destroy sheep corpses when constructing buildings
Reported by: | elexis | Owned by: | elexis |
---|---|---|---|
Priority: | Must Have | Milestone: | Alpha 23 |
Component: | UI & Simulation | Keywords: | patch |
Cc: | pathfinding | Patch: | Phab:D21 Phab:D1415 |
Description (last modified by )
When a sheep or other animal is killed and a building is constructed in the same place, gathering units will try to reach the corpse but never reach it. When constructing buildings, these corpses should be deleted.
Attachments (1)
Change History (21)
comment:2 by , 8 years ago
r17156 introduced the bug, before then the entity was destroyed.
Before that commit, the obstruction of the dead animal was:
obstruction: { active: true, blockMovement: true, blockPathfinding: false, blockFoundation: false, blockConstruction: true, disableBlockMovement: false, disableBlockPathfinding: false, shape: { type: "cluster" }
After that commit it was
obstruction: { active: true, blockMovement: false, blockPathfinding: false, blockFoundation: false, blockConstruction: false, disableBlockMovement: false, disableBlockPathfinding: false, shape: { type: "static", width: 2, depth: 2 } },
I have set the blockConstruction
flag in the XML part of the TemplateLoader
for dead gatherable animals, but that alone didn't do the trick. This is also the flag that the Foundation
component checks for in the Build
function (defined in GetUnitCollisions()
). The blockMovement
tag also wasn't it.
by , 8 years ago
Attachment: | commands.txt added |
---|
Sample replay for alpha 21 / r18804. The killed sheep is still visible and selectable in the middle of the fortress.
comment:3 by , 8 years ago
See logs on October 8 2016 for more information on this issue.
Git branch for testing: https://github.com/wraitii/0ad/tree/sheepFix
Should focus on putting various stuff under foundations and seeing how it reacts, and also check if walls and gates act sanely.
comment:4 by , 8 years ago
Keywords: | patch rfc added |
---|---|
Milestone: | Backlog → Alpha 22 |
Summary: | Destroy sheep corpses when constructing buildings → [PATCH] Destroy sheep corpses when constructing buildings |
- The flags should be accessible through the component, defined in JS, or it should become a boolean argument
comment:5 by , 8 years ago
Looks better with the most recent git commit, but I think that Obstruction.js
comment wasn't intentional.
comment:6 by , 8 years ago
Keywords: | roc review added; rfc removed |
---|
comment:7 by , 8 years ago
Keywords: | roc removed |
---|
comment:8 by , 7 years ago
Keywords: | beta added |
---|
comment:9 by , 7 years ago
Rebased branch, squashed into a single commit. Ready for review and commit.
comment:11 by , 7 years ago
Description: | modified (diff) |
---|
Style fixes:
- the PROFILE call has a typo in the name
- the split line in
GetStaticsOnObstruction
has bad indentation (coming from copy-paste), see the other function and copy that indentation. - remove the comment in
GetAllOnObstruction
- you didn't replace all occurrences in the public mod of
GetUnitCollisions
that you removed from the API
For me the big problem of this patch is this boolean passed to GetCollisions
which has an obfuscated name. It should be clear in the API what this boolean does (i.e. without having to understand the whole code in the Obstruction component).
I suggest calling the boolean onlyConstructionBlocking
(and probably making it optional, with a default value of true). Then comments should be detailed more in the function (explain in the first comment that living animals have the flag while dead ones are static obstructions without the flag). Add a remark that if the boolean is false, the filter filters nothing and is equivalent to NullObstructionFilter
.
Then I would add a comment in the Foundation.js code to explain that this code cleans everything under the foundation (corpses and other non foundation-blocking obstructions).
comment:12 by , 7 years ago
Keywords: | review removed |
---|
comment:13 by , 7 years ago
Description: | modified (diff) |
---|---|
Patch: | → Phab:D21 |
comment:14 by , 6 years ago
Keywords: | beta removed |
---|---|
Milestone: | Work In Progress → Alpha 23 |
comment:16 by , 6 years ago
Priority: | Must Have → Release Blocker |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
Everyone who had written or reviewed the patch (me, temple wraitii, Itms) haven't considered the flag 0
well evidently.
As reported by temple in Phab:rP21597, placing a foundation in the fog of war yields a foundation that is never going to be constructed as it is blocked by it's own mirage.
comment:19 by , 6 years ago
Priority: | Release Blocker → Must Have |
---|
comment:20 by , 6 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
I don't know if we have still the related bug: 'the ptolemaic camel archer kill a sheep wich was moving near the cc and then it tries to gather the corpse and doesn't suceed)".