Additional Texture Formats/Extensions Support by TDCRanila · Pull Request #25 · codecat/godot-tbloader

TDCRanila · 2022-09-15T18:50:16Z

Heya!

I was messing around with the tool, but noticed that textures other then '.png' were not supported. From what I can gather from the code, this is just because the texture file format when loading is hard-coded to '.png'. Changing the extension to '.tga', for instance, worked perfectly fine. So because of this, I have added support for other file extensions.

I decided to let the user control the array of strings/extensions, so that they order, remove, or add the ones they needed for their map. (Not sure if this really needed though and I am not convinced that is the greatest UI/UX experience, however, it works fine.)
Alternatively, we could:

Use booleans to represent the supported texture/image extensions - although it could clutter the code somewhat.
Just use the list of extensions that Godot supports via the "resource_loader->get_recognized_extensions_for_type" function instead and then remove the "Texture Import Extensions" property entirely. Although, it can happen that when the map is looking for a texture with an extension that is last on that extension list, it will waste processing time going through the others.
...or something else that I haven't thought of.

Anyhow, this is how you can use it in the editor.

additional-texture-extensions-support.mp4

I have also made a change regarding "texture_from_name" so that we don't have to; check for file extensions, check if it exists, and then call the load function for every texture. (This would be done twice for setting the texture size and then later for the mesh building.) It now fetches the resource from "m_loaded_map_textures" dictionary, which gets filled once at the start of map load.

Of course, feel free to suggest anything and let me know what you think. :)

…ts when building map/meshes. # - Users can add or remove texture formats using the exposed 'Texture Import Extensions' PackedStringArray variable in the editor. When loading, extensions are checked in ascending order.

…g them multiple times. # - Map textures are now attempted to be loaded during the start of 'load_map'. These textures are then cached in a dictionary so that they can be retrieved later.

…e textures cannot be loaded. # - Added additional error messages regarding this.

- Moved 'filter_nearest' functions to match the position like in the property inspector.

codecat · 2022-09-15T19:01:11Z

Thanks for the PR! 🎉

I like the idea of using Godot's supported texture list for this! We would need to check if all those texture formats are actually supported by TrenchBroom, too. I guess depending on that we can decide if we want to hardcode a specific supported set, or just use whatever Godot tells us it can use.

TDCRanila · 2022-09-16T16:29:59Z

Alrighty, I did some research,

Godot's Extensions List for "CompressedTexture2D" returns a list that contains the following supported extensions;
tres, res, png, bmp, hdr, jpg, jpeg, svg, tga, exr, webp, ctex, dds
Trenchbroom states in their documentation:

"The 'image' format can be used to load a wide array of image formats such as tga, pcx, jpeg, and so on. TrenchBroom uses the FreeImage Library to load these images and supports any file type supported by this library."

FreeImage Library Supported Formats

So theoretically, that would mean that all supported texture formats/extensions overlap besides tres, res, ctex, and svg, and thus means that whatever you are able to use in Trenchbroom, Godot will also be able to load it - using directory-based texture collections of course.

As for the PR, I would then indeed just use the Godot's supported extension list and then remove all the bits with the exposed property.
If we want to be more explicit with the extension list, it could be hardcoded, but I don't see much value in that since that Godot function exist albeit with extensions that Trenchbroom can't load.

On a sidenote, if we want to support more texture formats/extensions in the workflow, the 'textures' field in GameConfig.cfg that is provided with TBLoader, would have to be updated as well to accommodate this.

codecat · 2022-09-16T16:51:59Z

Thanks for taking the time to look into this!

I think I would have a preference for hardcoding the list to take into account the (unlikely?) scenario that Godot adds another format that is unsupported by Trenchbroom, or one that could be something other than an image (like tres). Then if someone needs another format we could just add it on request, since that would probably be a rare occasion as well.

And yes indeed, the default GameConfig.cfg would have to be updated as well. Since we don't generate this file yet, hardcoding the supported extensions also makes more sense to me.

TDCRanila · 2022-09-17T16:54:17Z

Yeah, no problem! ^^

I have removed the 'Texture Import Extensions' property entirely and replaced it with a hardcoded list instead like we talked about.

Let me know if there is anything else that could be improved or changed.

codecat

I just have a couple little style nitpicks, otherwise it looks great! 👍

codecat · 2022-09-17T20:17:42Z

+	const LMTextureData* tex;
+	String tex_path;


These 2 variables could be scoped inside of the for loop below, I think.

Yeah, that would work fine, but I thought since we are looping a lot for every texture and extension, it would be better to overwrite them instead of allocating/deallocating them constantly. However, if you prefer otherwise, that's cool too as it is pretty minor.

Agreed! Probably not super important for the pointer though, but for the string it does make sense.

codecat · 2022-09-17T20:18:07Z

+	String tex_path;
+
+	for (int tex_i = 0; tex_i < m_map->texture_count; tex_i++) {
+		bool has_loaded_texture(false);


This might look nicer? 😊

bool has_loaded_texture = false;

Oops! That slipped through, will correct that.

codecat · 2022-09-17T20:18:48Z

-			auto res_texture = texture_from_name(name);
-
+			auto res_texture = texture_from_name(tex.name);
+			


Some stray whitespace here

…instead of customizable. # - Reverted changes in tb_loader.h/.cpp, removing the "Texture Import Extensions" property. - Added hardcoded texture extensions list that both godot and trenchbroom support.

…ormats/extensions. # - Updating the GameConfig as tbloader now supports more texture formats.

TDCRanila · 2022-09-18T20:14:35Z

Alrighty, I have made the adjustments and have amended the commit, so that should be it. 🎉

codecat · 2022-09-18T23:05:21Z

Looks good to me! Thank you! 🎉

TDCRanila added 4 commits September 14, 2022 16:27

[Project, Tools] Now loading and caching map textures to avoid loadin…

eded26d

…g them multiple times. # - Map textures are now attempted to be loaded during the start of 'load_map'. These textures are then cached in a dictionary so that they can be retrieved later.

[Project, Tools] Now still loading the geometry of the map even if th…

2c9b4bd

…e textures cannot be loaded. # - Added additional error messages regarding this.

[Project, Tools] Style adjustments. #

023bb56

- Moved 'filter_nearest' functions to match the position like in the property inspector.

codecat suggested changes Sep 17, 2022

View reviewed changes

TDCRanila added 2 commits September 18, 2022 21:40

[Project, Tools] Changing the texture extension list to be hardcoded …

1c7beb7

…instead of customizable. # - Reverted changes in tb_loader.h/.cpp, removing the "Texture Import Extensions" property. - Added hardcoded texture extensions list that both godot and trenchbroom support.

[Resources] Updating Trenchbroom GameConfig to support more texture f…

0fb486b

…ormats/extensions. # - Updating the GameConfig as tbloader now supports more texture formats.

TDCRanila force-pushed the additional-texture-extension-support branch from dd881f0 to 0fb486b Compare September 18, 2022 20:06

codecat merged commit fca58df into codecat:master Sep 18, 2022

		auto res_texture = texture_from_name(name);

		auto res_texture = texture_from_name(tex.name);

Uh oh!

Conversation

TDCRanila commented Sep 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecat commented Sep 15, 2022

Uh oh!

TDCRanila commented Sep 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecat commented Sep 16, 2022

Uh oh!

TDCRanila commented Sep 17, 2022

Uh oh!

codecat left a comment

Choose a reason for hiding this comment

Uh oh!

codecat Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

TDCRanila Sep 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecat Sep 18, 2022

Choose a reason for hiding this comment

Uh oh!

codecat Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

TDCRanila Sep 18, 2022

Choose a reason for hiding this comment

Uh oh!

codecat Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

TDCRanila commented Sep 18, 2022

Uh oh!

codecat commented Sep 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

TDCRanila commented Sep 15, 2022 •

edited

Loading

TDCRanila commented Sep 16, 2022 •

edited

Loading

TDCRanila Sep 18, 2022 •

edited

Loading