Portponky / gridmap-plus

Godot plugin to give some love to gridmaps.
The Unlicense
16 stars 1 forks source link

Added lighting options that can be configured from the Dock #2

Open diegopau opened 1 week ago

diegopau commented 1 week ago

Hi!

This is the first pull request I do not just to this repo, but to any repo, so please forgive me if I miss anything.

Also I am pretty new to Godot so I might not be following the best practices in the code I added. The changes have been done in part with the help of Claude. For these reasons please feel free to completely discard this pull request or completely change whatever you feel like, without any worries.

What I added:

Limitations: The lighting settings are not saved. Whenever Godot is restarted all the configuration defaults again to its default values.

Implementation detalis

Why was this needed?

In my case my game is going to be set in mostly underground caves and similar interior environments. In that kind of setting a directional light is not ideal and for my map scenes I will be using OmniLight3D nodes so it would be very helpful to achieve similar lighting conditions while in the editor, else the colors can be dramatically different. I believe it can be useful for other creators too.

However I have to say that this still didn't solve my use case, but I think this is due to a different addition that would be needed complementing this one, so I wanted to keep them in separate pull requests. I anyways explain what is happening to me specifically:

Currently, even with a DirectionalLight3D added to my scene (when just using the Godot editor) I can see my scene with a decent color representation of what the in-game result should be. However when using a very similar lighting in the GridMap+ builder, all I see are the walls that are directly hit by the light, but not other walls (that show 100% black). Not even playiing with angular distance or other parameters can fix this. Here you can see the difference: result

in the Godot editor I have ideal results using an OmniLight3D: image and with an OmniLight3D, with similar parameters, in GridMap+ I get this: result_omni

I believe this is now related with the rendering and environment options in the viewport that is created with the builder window. Now those options are probably set to the most basic values, which is probably the best for most use cases, but in my case it would benefit from enabling a few options. If this pull request gets greenlighted I can later on add another pull request for adding those extra rendering/environment options for whoever needs to enable some options at the cost of a performance hit as it seems to be my case.

Looking forward to your comments.

Portponky commented 1 week ago

Hi, I'm away this weekend (and next) but I will make some time to look at this mid-week. It looks really good! I don't do much 3D so I didn't even think about interior scenes.

One thing that might be worth doing is implementing the lighting options as Godot project settings options, so they instead show up in the Project Settings menu and are saved between editor sessions. I have done this with another plugin I made here https://github.com/Portponky/better-terrain/blob/1c66c9ce43d8e5150f28e532e0f6b8e983e4734f/addons/better-terrain/editor/Dock.gd#L131 - it's a little bit longwinded but reasonably simple.

diegopau commented 1 week ago

No hurry at all with reviewing this!

Meanwhile... after reading your message I got a bit carried away with future possibilities, based on needs of my project that I believe would apply to other people whose game rely heavily on Gridmaps. I apologize in advance for adding more discussion to the pull request but since it is related I thought I should share my thoughts and get your feedback on it.

Adding lighting options as Godot project settings

While I do think this is a great idea for things like the different control schemes, maybe UI options that could be added in the future. For my specific needs having the lighting settings in the Godot project settings would be a downgrade because they are harder to access and I think I will be tweaking the lighting settings frequently (in real-time) to get the best lighting for a specific scene (imagine a very curvy cave with areas that are not reached by the light, then you need to move the light to different points as you do edits).

Right now, you have to click on the GridMap node and then in the GridMap+ tab of the dock and then click on "Enter Build Mode" which opens a new window. And with that done, all the lighting options, and other similar options we might add, are already there, in that same dock that you can have near the Builder window. I think this is much more convenient than having to then open the Godot Project settings and search for the GridMap+ settings.

But the actual main reason why I think they might be better in the dock is related to a kind of vision I have for all this, that I would not mind implementing myself if you think this is a good idea, thought it would take me quite some time I think. The vision is saving several settings in a per-gridmap basis, trying to use a simple approach. It needs a bit of explanation...

Saving settings per Gridmap.

In a large project that is based on Gridmaps, I think it would be great to save some of the options per each Gridmap. This will include for now the lighting options and the environment options I talked about previously. For the environment options I think it will be best to just specify an environment .tres file which people can configure in a way that they can have one per gridmap or one per group of gridmaps, whatever they prefer. Then we just attach the environment as a child node of the Builder window (and Godot should automatically grab that way the environment settings, I think). image

So in your project you might have underground levels, interiors, exteriors, levels in space, all of them using gridmaps but with different lighting and environment needs to make the editing more comfortable. I think it would be great to just save the settings for each gridmap in a way that they autoload as soon as you click in the related Gridmap and when you are in the Dock, instead of saying "General Options", it could say "Gridmap Options for {Gridmap_Name}" to make it clear that those are for the specific Gridmap.

Using a plain text file to store the per-gridmap settings

My preference would be to store to the settings in a simple text file (but using a structured file format, like json).

What to use to identify each gridmap?

Depending on what you do with Gridmaps and Meshlibraries on Godot you might have realized that they are a bit "delicate" in the sense that as you project grows you most likely need to do constant changes in your MeshLibraries (add or remove meshes, which can end up changing the IDs of the meshes), then when you export your Library as MeshLibrary with the changes you need to update the GridMap with the updated MeshLibrary because it just doesn't refresh it automatically, and in fact, at least in my case, I have to remove the MeshLibrary association from the Gridmap node (in the inspector), restart Godot, add the library again. More over, in my case, I even often regenerate the Gridmap, because I have a script that generates an initial gridmap from a .ply file (exported from MagicaVoxel) and until I am happy with that initial gridmap I might rerun the script several times (this is quite a specific situation, but in large projects all sort of needs arise).

So all of this is to say that, at least in my project, the MeshLibrary is constantly being recreated and even the gridmap can be recreated. It would be nice not to lose the settings if those things happen.

So as an initial idea I think a way of doing this is to store the settings in a file by identifying each gridmap with a string that is composed by the scene file path + the node path inside the Scene. For example in the case of my node it would be "res://scenes/0001_initial_cave.tscn/GridMapGenerator/GridMap". This way even if the gridmap is recreated the settings are still there and will apply. If the gridmap is moved or renamed, it would be still very easy for the user to go to the settings file and manually change the path so it matches with the new path.

image

In a related topic if I understood the code correctly, for the Hotbar shortcuts the data is saved as metadata in the meshes which is great but that I imagine the shortcuts are lost after re-creating the MeshLibrary (I didn't test this). I still don't really know if it is just me doing this wrongly, but it really seems like as long as you are adding and changing stuff to the MeshLibrary you would be recreating it often. So in theory we could also save the shortcuts in the same file (this time just identifying the MeshLibrary with its path) and re-applying the metadata to the meshes if we detect that the MeshLibrary does not have any existing shortcuts but we do have them saved in the settings file.

Conclusion

Even if you like my proposal, before I even start working on any of that I would like you to first review this pull request and see if I am approaching this correctly, but I thought it would be good to bring the possibility of that per-gridmap settings system since you mentioned the possibility. Sorry for making this so long!! thanks for reading.