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

WIP: Ocean System #385

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

anandbarai09
Copy link

@anandbarai09 anandbarai09 commented May 26, 2024

Todo
Ocean Shader(In Progress, Current Shader is not good looking and has many problems),
Ocean Physics.

Done :
Ocean Clipmap
Ocean3D (Ocean3d node , Ocean3DStorage, Ocean3DMaterial) (will require some cleanup)

@TokisanGames
Copy link
Owner

Ocean Clipmap looks identical to GeoClipMap. You should just use GeoClipMap to generate a second clipmap. We put it into a separate class so it can be reused. If there are particular custom options you need in the ocean mesh, GeoClipMap can be modified.

@TokisanGames TokisanGames marked this pull request as draft June 1, 2024 10:09
@TokisanGames TokisanGames changed the title Ocean System (Work in progress) WIP: Ocean System Jun 1, 2024
@anandbarai09
Copy link
Author

ok I will create a new function in generate_ocean() because it uses skirt mesh and terrain3d's clipmap does not use skirt.

@JekSun97
Copy link

JekSun97 commented Jul 6, 2024

@anandbarai09 It would be helpful if you could share screenshots

@TokisanGames
Copy link
Owner

I'm sure screenshots can wait until Op has something implemented and working.

Op, like my comment on GeoClipMap, you're duplicating a lot of classes. That is unnecessary. Change the existing classes or derive from them as needed. Reuse code religiously. Utilize the inheritance model within C++. You can dramatically reduce the amount of code, which will make this PR easier to write and maintain as you get closer to completion.

Even the change you made to GeoClipMap is unnecessary. You don't need to duplicate an entire function just so you can have skirts. Instead add a parameter to the function and add skirts if that parameter is true. Your 400 lines of changes to this class will be reduced down to 20.

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.

3 participants