Adds Automatic Building Functionality to Map Nodes

[Pspritechologist]

Requiring a manual press of a 'build' button after each map change to see the effects within Godot becomes tedious quickly, not to mention requiring that you manually hunt down each map Node one at a time to do so.

This PR adds an auto_reload property to map Nodes allowing them to react to the reimporting of a map file, making working with them very similar to working with .blend files directly.

[Pspritechologist]

Pretty new to the repo, but I think this tool is fantastic! When I found that I'd need to hit 'build' after every map change, I already knew I'd need to implement something to change it. When I found that I could implement it so easily though, I figured I may as well see if anyone else found it useful as well :P

[RisingThumb]

This will build every single time this scene is opened in the editor, which means you'll be hit with a lag spike every time you open a map as it builds. Can we do something else here?

[Pspritechologist]

If you'd like, but that's part of the point for me. If you've got a particularly large map that you don't want to generate every time you open it, you can simply turn the bool off for it.

Without getting far more complicated with it, there's no way to check if a Resource has been modified since the last time a Scene was open, and so simply rebuilding each time it's opened seemed minor to me in favour of it being up to date. I was impressed with how performant the plugin has been since I started using it, but I have only used extremely simple maps.

On a side note, this PR doesn't deal with the requirement to open each Scene containing a map file you've modified each time you alter them. Batch editing maps seems like it could grow to be a huge pain, but I don't know a solution personally. You'd need to find map Nodes and load the Scene up in the background before re-saving the file each time the Resource is edited, which sounds a little sketchy. Funnily enough, it'd also solve the problem listed here :P

[RisingThumb]

Perhaps it should be moved from a func_godot_map field, to a func_godot_map_settings field, this way it applies to all the maps using these settings

[Pspritechologist]

I considered that, the trouble is that it's far more per-map considerate than any other setting in that Resource.

[RisingThumb]

Perhaps... perhaps we can have one in the map settings that if set, overrides it for all maps using it as a setting? Ultimately, I'm not too fussed about it

[RhapsodyInGeek]

I had thought about suggesting it be in map settings, but honestly I feel it fits more on a per-map basis rather than a gamewide basis. I'd have to agree with Psprite's judgment in this specific case.

[RisingThumb]

I'm not 100% convinced. Currently a lot of the settings that you could argue are per-map, are in these settings. Stuff like unshaded, save generated materials, use trenchbroom groups hierarchy etc. It also makes an opinion that map settings are gamewide, which I also don't think is always the case(unshaded is a good example of this!).

• You want to use dev settings for development("mat_fullbright 1" for unshaded) and production settings for prod
• Reloading only props that are being worked on(dev/prod idea, but with prod you don't want runtime map loading in your finished exports. Not everyone wants to expose dev tools in their finished games)
• Toggle on for the map setting, open all scenes using these map settings to regenerate them, and then toggle it off in the settings(as opposed to turning it on and off in each FuncGodotMap scene if they aren't using a premade FuncGodotMap scene, or just clicking the build button). If you have to turn it on manually for all maps or build manually for each, that will get tedious fast

While I agree it's more per-map specific, so are others like unshaded and use trenchbroom group hierarchy

[RhapsodyInGeek]

Fair assessment. The original intent of a map settings resource was to share setups across maps without having to re-do them every time or create template scenes that you might accidentally "corrupt". A side benefit of this was using the same map with different settings could yield different results, and a way to apply it to the auto_rebuild / auto_uv_unwrap features would be to have a "production map settings" and a "release map settings", or some other combination that you can hot swap in and out.

While you could have an additional "override" variable that dictates whether an auto rebuild occurs or not, I'd lean on the side of "not". While we do have some overrides scattered about here and there, we got rid of most of the ones that were found in Qodot as they were bandaids to avoid breaking projects.

I'd have to defer to @RisingThumb in this case, the auto rebuild setting does belong in the map settings resource given what it's used for already. Shouldn't be all that difficult to change over though.

[RisingThumb]

This functionality looks really useful, especially for people who have a workflow going in and out of the editor 👍

[RhapsodyInGeek]

It's a neat feature, probably useful for some folks.

• The variable should be renamed to auto_rebuild_on_local_map_update. It's a lot more verbose, but the verbosity also tells us exactly what it's supposed to do, and its limitation.
• The default value should be set to false.
• Description probably better as ## If true, automatically rebuilds the map when the local map file is reimported after being saved externally.

Saw a question about the purpose of global map files. There are actually a lot of use cases for them, particularly for runtime building / modding. I do not see the auto-rebuild feature needing to support that though, so the limitation to local files is perfectly acceptable as long as we tell users that's a limitation.

Any issues if you don't have a map file set?

[Pspritechologist]

Saw a question about the purpose of global map files. There are actually a lot of use cases for them, particularly for runtime building / modding. I do not see the auto-rebuild feature needing to support that though, so the limitation to local files is perfectly acceptable as long as we tell users that's a limitation.

Any issues if you don't have a map file set?

Ah, runtime loading makes a ton of sense for global path!

An imported Resource will never equal an empty string, so should be no issue at all.
Even if it could trigger an auto build, it should be about as problematic as clicking the build button on a Node with no map file set.

[Pspritechologist]

Web edits so I don't need to push 😎

[Pspritechologist]

It's a neat feature, probably useful for some folks.

• The variable should be renamed to auto_rebuild_on_local_map_update. It's a lot more verbose, but the verbosity also tells us exactly what it's supposed to do, and its limitation.
• The default value should be set to false.
• Description probably better as ## If true, automatically rebuilds the map when the local map file is reimported after being saved externally.

• I've used the word 'import' frequently rather than 'update' to more directly clarify what triggers the process.
• Honestly speaking, it was meant to 'false' when I first made the pr :P. I of course prefer the functionality to be default which is why I had it otherwise locally, but I'll just set that when I use it myself. 👍
• Does this description seem acceptable to you? It's quite a bit more verbose but I enjoy explaining the details of what
  something is, and why the user might care about it, since it displays as the tooltip. I'm happy to shorten it to what you have if you'd prefer.

[RhapsodyInGeek]

I've used the word 'import' frequently rather than 'update' to more directly clarify what triggers the process.

Should still be 'update', as that's the practical application, what's going on in the layman sense: they updated their map, so the FuncGodotMap is automatically rebuilding it. Also, should be specified as "local map file" (or just "local map") because that's much more clear than just calling it "local file".

Does this description seem acceptable to you? It's quite a bit more verbose but I enjoy explaining the details of what
something is, and why the user might care about it, since it displays as the tooltip. I'm happy to shorten it to what you have if you'd prefer.

Too much, most of it's superfluous to be honest. The description I gave you tells everything the user needs to know about what it does in a succinct way. The member variable descriptions shouldn't enter the realm of tutorials. Descriptive, but brief.

[RisingThumb]

Just to add, this comment is still outstanding. All the other changes look good to me, thanks for addressing these! 👍
#13 (comment)