decline-cookies / anvil-unity-dots

Unity DOTS and ECS specific additions and extensions to Anvil
MIT License
4 stars 1 forks source link

AbstractTaskDriver - Assert subtask instance has no parent. #204

Closed mbaker3 closed 1 year ago

mbaker3 commented 1 year ago

Add an assertion to AddSubTaskDriver to make sure the subtask instance hasn't already been added to another Task Driver.

What is the current behaviour?

Though it will produce incorrect behaviour it is possible to add one subtask driver instance to multiple task drivers (or even the same task driver multiple times)

What is the new behaviour?

An assertion will be thrown when a subtask driver instance is added multiple times.

What issues does this resolve?

What PRs does this depend on?

Does this introduce a breaking change?

mbaker3 commented 1 year ago

Can open a new PR to adjust if we decide to change but to me this is a super cheap check that will only come up which you're in the process of adding subtasks and developing this feature. It's not like an edge case behaviour at runtime will suddenly trigger this condition.

So a message isn't required. It's pretty apparent what's gone wrong if this triggers. In terms of ANVIL_DEBUG_SAFETY vs DEBUG, really if there was an editor specific assert I would have used that because this is never going to happen in a build if you've already tested in the editor.

Rationale

I vote we leave it as is.

jkeon commented 1 year ago

🚚 💥 🚲 💀