ItsKorin / Godot-Post-Process-Plugin

A simple godot 4.2 plugin that adds shader based "post processing"
MIT License
252 stars 17 forks source link

Creation of effect presets and code main loop optimisation/cleanup #4

Closed Loufe closed 5 months ago

Loufe commented 5 months ago

Please note this IS a breaking change, though a minor one. I believe it's worth integrating because the modularity of presets as resource files I think would be very useful for many people's projects. IMO it should be an easy change for anyone hoping to use the add-on.

I really like your project, Korin, thanks for making it. It perfectly fit our niche for needing a simple and straightforward way to configure post processing effects without needing to get lost in learning shaders. Thanks for publishing it!

As I started implementing it deeper into my project, I realized I wanted to create preset effects I could define in resource files (and configure in the editor and not in code), like I do for NPCs and items, etc. It wasn't possible as the Canvas Item can't be put as an export of a resource, so I thought I'd implement the change and propose you integrate it into the project.

This change splits off export settings to a new class (PostProcessingPreset) which can be made into resource files (just attach the class script to an empty resource, like in the two example resources). The main class now takes as a parameter one of those "preset" resources (or one created INSIDE the post class instance), either done in a fixed manner in the scene tree (like before) or dynamically with the "inside" type.

image

I also tried to reduce the boilerplate code and find optimisations where I could. I know there are a lot of changes, but it is functionally exactly as before, just hopefully a little easier to read and a little quicker to run. You did a great job structuring the project, these were just some QOL touches.

I tested a bunch and everything seems to work well now. If you have any questions or want changes, don't hesitate, I'd be happy to work with you further.

What do you think, @ItsKorin ?

ItsKorin commented 5 months ago

Hey @Loufe, I was in fact thinking recently on adding presets, so you saved me a lot of time (and optimised the heck out of my code haha).

I have one question tho, i'm still pretty new to the opensource scene, so how would this work, any proper attribution or something? I'm also open to new ideas and suggestions

Loufe commented 5 months ago

Hey @ItsKorin , thanks for being open to my PR! Not all maintainers respond well to big changes so it's much appreciated.

Please feel free to review my code (using the built in tools on github here) if you have any questions / want changes. I'll happily respond.

As for attribution, this is your project, you'll be the one maintaining it, IMO you should still be the figurehead. I don't need to be explicitly named anywhere, unless you really want to name contributors, but personally the "contributors" section is enabled on the repo, which is good enough for me. A classy example : Nathan Hoad's dialogue manager is a gold standard for Godot add-on setup, and he has a nice succinct "contributors" section in the readme if you're looking for inspiration.

By the way, the CC BY 4.0 licence looks like a decent permissive licence, dunno if you were set on it, but I did some quick reading online and it seems like people prefer MIT for software.

Also, if you have discord and wanted to connect, I'm loufe#6923

ItsKorin commented 5 months ago

bet

Thank you for taking your time to enhance the project! Regarding attribution, i will create a section in the README, and (becouse i can) in some comments in code.

As for the licensing, i'll look into the MIT licence and see if it would work for the project. (it propably will)

If you have any more ideas, suggestions or bug reports, please don't hesitate to share them.

Worm Regards, Korin