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

Explore what multiple ocean instances would mean #650

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

huwb
Copy link
Contributor

@huwb huwb commented Sep 12, 2020

Status: Definitely not ready to merge. I'm creating this as someone enquired about it and it took me a while to find it.

Experiment with what would be required for multiple oceans: #540

This branch should work like before in the case of a single ocean. There are a few issues before it will support multiple instances.

Questions / problems that arose:

  • RegisterLodDataClipSurface - this sets per-ocean data on MPB. might be tricky to fix..
  • How to connect underwater effect to ocean instance?
  • How to connect underwater environment lighting with ocean instance?
  • OceanChunkRenderer.OnWillRenderObject - tiles will need Layer so it only goes in one camera??

Misc notes:

  • Validation gets passed "AnyInstance" in a few places, is this ok?
  • RenderData.Validate - uses any instance
  • Debuggui always takes first instance of ocean, shall we make this selectable?
  • Lod data resolution should be global setting, right now taken from one instance

Once i started pulling on the thread i could not stop. This branch should work like
before iwth one ocean, there are a few issues before it will support multiple instances.

Questions / problems that arose:

Lod data resolution should be global setting, right now taken from one instance
Validation gets passed "AnyInstance" in a few places, is this ok?
RenderData.Validate - uses any instance
RegisterLodDataClipSurface - this sets per-ocean data on MPB. might be tricky to fix..
How to connect underwater effect to ocean instance?
How to connect underwater environment lighting with ocean instance?
Debuggui always takes first instance of ocean, shall we make this selectable?
OceanChunkRenderer.OnWillRenderObject - tiles will need Layer so it only goes in one camera??

tag @daleeidd @moosichu in case this is of interest
@daleeidd
Copy link
Collaborator

daleeidd commented Oct 28, 2020

I have the following assumption:

Each camera can have many ocean instances.
Each ocean instance must have only one camera.

Going with the simpler assumption of a one to one mapping with camera and ocean.

How to connect underwater effect to ocean instance?

We could do a camera lookup. It would require #677 so that we have a reference to the camera. Done

How to connect underwater environment lighting with ocean instance?

Same as above. Although, developers would have to use light layers and have two separate lights for it to work. I think we should instead add validation stating that this component doesn't support multiple instances. And maybe add support in the future.

OceanChunkRenderer.OnWillRenderObject - tiles will need Layer so it only goes in one camera??

We already have this for built-in. I think SRP uses something different. We could validate this if we have a camera property.

Validation gets passed "AnyInstance" in a few places, is this ok?

We should be passing all instances. Since these components are shared by both instances, they should be valid for both instances. The validate function will need to be updated to take list of OceanRenderers. I will look into doing this at some point.

daleeidd added 2 commits May 13, 2021 17:01
Validation already enforces a camera as a parent.
Multiple Oceans: Connect underwater to ocean instance
@daleeidd
Copy link
Collaborator

daleeidd commented May 24, 2021

Updated above comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants