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 crate spawned actors not crushing crushable actors and spawning inside other actors. #20849

Merged
merged 2 commits into from Aug 8, 2023

Conversation

PunkPun
Copy link
Member

@PunkPun PunkPun commented May 7, 2023

I fix it by pushing cell selection logic into end world task, and by calling SetPosition after the actor was spawned

@PunkPun PunkPun changed the title Fix crates spawned actors not crushing crushable actors and spawning inside other actors. Fix crate spawned actors not crushing crushable actors and spawning inside other actors. May 7, 2023
@anvilvapre
Copy link
Contributor

I fix it by pushing cell selection logic into end world task

Side question. This seems like a fix for more and more things. Which makes sense. But if so much tasks become frame end tasks, being executed after a logic tick, won't you have the frame end tasks competing as well.

@PunkPun
Copy link
Member Author

PunkPun commented May 11, 2023

I don't think so. End frame tasks are mostly there to handle object creation as to avoid modified collection during enumeration crashes. And as it seems much of the older code processes creations checks at the wrong time. It's not unlikely that some major refactor screwed a lot of code up.

Fwiw I've also seen end frame tasks used for multithreading.

@abcdefg30
Copy link
Member

Does that mean we can just put the SetPosition part into the frame end task?

@PunkPun
Copy link
Member Author

PunkPun commented May 11, 2023

I'm not sure what exactly you mean. But I don't really see a reason why we can't put SetPosition there.

@abcdefg30
Copy link
Member

I was asking if we can just put the SetPosition stuff inside the task and leave the rest outside.

@PunkPun
Copy link
Member Author

PunkPun commented May 11, 2023

No, then none of the bogus behavior would be fixed

Mailaender
Mailaender previously approved these changes May 19, 2023
@Mailaender
Copy link
Member

Can you remove the test case?

@PunkPun
Copy link
Member Author

PunkPun commented Aug 8, 2023

removed

@Mailaender Mailaender merged commit 54dac39 into OpenRA:bleed Aug 8, 2023
3 checks passed
@Mailaender
Copy link
Member

Changelog

@PunkPun PunkPun deleted the fix-crate branch August 8, 2023 14:08
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.

None yet

4 participants