Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix carryall not removing influence when cargo dies #20490

Merged
merged 4 commits into from
Jul 25, 2023

Conversation

PunkPun
Copy link
Member

@PunkPun PunkPun commented Nov 28, 2022

Fixes #20468
An oversight from #16509 and then #20403

Here we make sure we properly handle cancelling regardless of where it was called. We cancel all child activities then set ChildHasPriority to true and IsInterruptible to false so that the Takeoff would run regardless of PickupUnit code and condition.

@PunkPun PunkPun added this to the Next release milestone Nov 28, 2022
@PunkPun PunkPun force-pushed the fix-carryall2 branch 2 times, most recently from 9766921 to b498e3b Compare November 28, 2022 21:35
@PunkPun PunkPun force-pushed the fix-carryall2 branch 2 times, most recently from 209de5b to 5755c27 Compare November 29, 2022 17:09
Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually marking the TakeOff activity as uninterruptible doesn't seem right. When nothing inside the activity itself goes wrong, but the player decides they want to have a carryall that has descended to pick up a unit to stay landed, they cannot prevent it from taking off. That makes me think that #20403 was the incorrect fix to begin with. (And #20401 as well, unfortunately both PRs were merged with basically just one +1. The real bug is the state Land leaves us with.)
#20212 just exposed issues with the aircraft landing code which probably didn't work properly for quite some time already. I concede that fixing that is out of scope for the next release though, so I think we can take this PR.

OpenRA.Mods.Common/Activities/PickupUnit.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Activities/PickupUnit.cs Outdated Show resolved Hide resolved
@PunkPun PunkPun force-pushed the fix-carryall2 branch 4 times, most recently from 9388904 to 00a1809 Compare November 29, 2022 20:21
@abcdefg30 abcdefg30 modified the milestones: Next release, Next + 1 Dec 1, 2022
@Mailaender Mailaender changed the title Fix caryall not removing influence when cargo dies Fix carryall not removing influence when cargo dies Mar 18, 2023
@PunkPun PunkPun force-pushed the fix-carryall2 branch 5 times, most recently from 29f9aaf to 193cf0f Compare March 18, 2023 15:22
@PunkPun
Copy link
Member Author

PunkPun commented Mar 18, 2023

added more documentation

Copy link
Member

@penev92 penev92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

@penev92 penev92 merged commit c7e0bc4 into OpenRA:bleed Jul 25, 2023
3 checks passed
@PunkPun PunkPun deleted the fix-carryall2 branch July 25, 2023 07:26
dnqbob added a commit to dnqbob/OpenRA that referenced this pull request Jul 25, 2023
MustaphaTR added a commit to MustaphaTR/OpenRA that referenced this pull request Jul 25, 2023
Add OpenRA#20490 to fix carryall crash, with rebase mess fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add influence about land, again.
4 participants