Water body auto clip-include

[huwb]

Make waterbody automatically clip-include

If clipping is enabled and clipping set to remove everything by default, then the waterbody ensures its area is clip-included.

This seems good to me, but needs the ocean lifetime stuff to ensure OnEnable is called after the OceanRenderer.

No urgency on this so ill just park it here.

NOTE: Just realised i created it against master instead of local-water-body. I'm guessing the life cycle stuff won't merge soon so this is probably fine, but review-by-commit is necessary, just the last one is relevant.

[daleeidd]

Nice! An option to disable auto clipping will be useful. Currently if I want to create a sphere, I will have to clip it with a box and then unclip it again with a sphere.

[huwb]

You mean you are setting to clip everything by default, then adding a box to include, then a sphere to cut out again?

[daleeidd]

You mean you are setting to clip everything by default, then adding a box to include, then a sphere to cut out again?

Image below will help. I want to clip by default, and then use a sphere to include water. And use the water body too. I would have to undo the clipping the water body adds to accomplish this.

[huwb]

Yeah fair point added now

Also merged master so that diff is not horrendous

[daleeidd]

Looks good. Just a couple of things.

[daleeidd]

Nice work.

[daleeidd]

I missed this but the execution order will need to be after the ocean for it to work in builds. And release notes if you want. Can merge after that is done.

[daleeidd]

I did the last review items. Ready to merge when you're ready.

[daleeidd]

@huwb happy to resolve release note conflict and merge if you're fine with everything.

[huwb]

thanks for bumping

what is the requirement for the update order? i'm not super against it but just curious if we can avoid it. the update order adds implicit dependencies between parts of the code which are not documented and can break, so previously i did a push to minimise the use of update orders.

perhaps an alternative is to call our own update function on all waterbodies from the OceanRenderer component. however im aware this is editor code and maybe not worth it. but it might be good to record what the update order dependence is in a comment.

[daleeidd]

what is the requirement for the update order?

The Update method is stripped from builds. So this feature will only get triggered from OnEnable in builds which is execution order dependent on OR.

but it might be good to record what the update order dependence is in a comment.

Done.

perhaps an alternative is to call our own update function on all waterbodies from the OceanRenderer component.

Agreed. Going to look at doing that next. I am considering using C# delegates or would you prefer we maintain our own registry and call directly?

[huwb]

Ohh it's OnEnable. I guess this is part of the same update issue we're discussing oh the other pr that ensures events get called at the right times when OR enabled/disabled. Do you know which PR I mean? I'll see if I can find it after posting this.

[huwb]

Ok yeah #629 . ideally we could converge that rather than workaround it here. I'll try to reply there shortly.

I'd be fine with the changes as is in the meantime - using update order - but would be good to revert once #629 merges. or we could try to get #629 done first.