Automatically export scenes to display them in Trenchbroom #95
Conversation
gwenvis
changed the title
|
One thing to note is that This bit here in the Model Point Class resource's var path = "addons/qodot/export/" + scene_file.resource_path.replace("res://", "") + ".glb"If I'm reading it right, you're creating the exported GLTF in a new folder with the Qodot addons. You should never have user created files in an addon's folder since you can't guarantee the addon was downloaded from Asset Library. If it was then they will lose all the files they added to it. And even if it wasn't, it's not recommended because if an addon update removed certain files then you'll have a harder time updating it. User created files should always be outside of the addons folder. This setup also forces your TrenchBroom game folder to be your entire project folder still, which reduces flexibility. You should instead have an exported model path variable rather than rely on a hardcoded one. You should also look at trying to rotate the model in the scene file first, if that's possible. Otherwise you'll still have angle problems in TB. At this point though, it's feeling like a bit more work to do it this way than just simply using an export template for your model in Blender and adjusting a boilerplate model string in a regular Point Class resource's Meta Properties. That string by the way is just I'm not saying "give up" by the way. Just that for this to be a viable method of doing things you'll want to make it more convenient than the current way of pulling it off. P.S: Credit where credit is due, your code style is very clean and made all of this very easy to read. |
|
@RhapsodyInGeek Thank you for the quick comment! Your advice is appreciated, you know more about this than I do.
I'm not, and that's why this is a draft. At the moment I haven't covered all edge cases and I'm still figuring out how to do this properly, but what I have commited works so far.
My mistake! I mainly used
The reason I added it to the addons folder is because they're generated files. It doesn't make sense that any of the files generated are commited to any source control because the one editing the levels has to export the fdg files anyway. The files there are only generated for representation in Trenchbroom, that's why there's a I could of course make this a user defined property, and that's something I want to do, but it's important (or rather, useful) that Godot and git ignore the files generated.
That's something I want to add, but not before having other people giving advice on what I've made.
Possible! Rotations are correct when Qodot builds the map because I apply the proper transformation there, but the original scene file differs by 90 degrees. Rotating the node before exporting is a great idea, I don't know why I haven't thought of that :p.
I mean, doing all that work for every model can be quite time consuming. Mine works (and is optional) completely automatically, so none of that is required. This is an easy solution for future users :). |
I was thinking if you use the custom export path and then if there are no
Instead of changing the
It's not actually. You do it when you're exporting the models and save it as an export template in Blender. Super quick, super reliable, doesn't take that much time especially if you're already working with the model and have it as part of your flow. Maybe a minute in Blender to do the work tops. The biggest hurdle with the current method isn't really the "work" you need to put in it, it's a knowledge and familiarity issue that users have. I can see this being useful if you're using models you didn't make and have no Blender knowledge, with some changes to the design to allow customizability, compatibility, and safeguarding the user's work (some of which you've mentioned already looking to implement). |
I could always force generation to use a subfolder and put the .gdignore in there. So if you specify Or there could just be some documentation somewhere suggesting the user to add those files themselves if they wanted to.
I mean I guess. It's not something that is very intuitive. You'd have to export 2 models everytime you make changes. It's trivial and once you have the models set-up it's barely any work because everything already exists but it's much more streamlined if Qodot were to do it automatically. You'll always have 2 files in your project. One optimised for Trenchbroom and the other for Godot. Mine technically has the same, but it's not (or shouldn't) commited to source control, and also optional for people who don't level design the levels. It's just fast, and like you said doesn't require Blender knowledge and you can easily add downloaded assets to the mix if desired (which is pretty important). Having this not be a hurdle people have to go through would improve Qodot much more.
Oh smart, I'll try that tomorrow. Might work if it sees an absolute path. I was thinking it might work in the Trenchbroom game config folder as well, but probably not. If anything, I at least want to merge some changes into Qodot that makes it easier for me to make the entity stand on it's own. (So people can decide whether they want it or not) Your GameConfig change seems really useful and would solve the problem of having to set the path for every entity you create, so I'll wait on that. |
I think you misunderstand the workflow. You only would need to export the TrenchBroom version of the model the one time, unless you make significant changes to the mesh appearance. For every time you export it for having new animations, you wouldn't need to export the TB version again. I would agree it's not intuitive, but honestly at least half of (if not more than) the work flow to make effective use of Qodot isn't. Once again, it just comes down to not a matter of the amount of work but rather a matter of education on it. Hence why I took the time to make a huge tutorial on the plugin for people (at least so they can shut up about "Qodot only being good for prototyping lol).
I don't know why you think the TB models shouldn't be committed to source control. Maybe you mean not imported into Godot? Those would be two very separate and very distinct things (the I don't disagree it'd be nice to have Qodot have the option of doing it automatically. I do think you need to reduce the amount of assumptions your proposal makes though. One of the biggest strengths / pillars of Qodot is that it doesn't make assumptions about what you want: you have to define it yourself. Hence all of the different folders you have to set, the fact that the only key values it auto-implements are the bare minimum to generate the mesh ( Which reminds me, I'd recommend a In any case, with that design pillar in mind, I do think it was a good decision on your part to make the Model Point Class an extension of Point Class rather than a modification of it.
I think you meant "having to set the scale"? Because that's all I added to GameConfig. In any case I think this solves everything on the resource side except your automatic size calculation (it seems to be broken, but you don't typically want to have your entity size determined by your AABB anyway for a lot of reasons, particularly animated entities). @tool
extends QodotFGDPointClass
class_name QodotFGDModelPointClass
## The folder that matches your game's directory in TrenchBroom's preferences
@export_global_dir var trenchbroom_game_dir: String = ""
## The models sub-directory within your trenchbroom_game_dir
@export var trenchbroom_models_dir: String = ""
## Scale expression applied to model in Trenchbroom. See https://trenchbroom.github.io/manual/latest/#display-models-for-entities for more info.
@export var scale_expression: String = ""
## Most of the time your actual entity bounds won't be the same as your AABB.
@export var enable_aabb_size_calc: bool = false
## Entities auto-rotate by default based upon mangle, angles, or angle key value. Set to true to disable this behavior, in case you don't want the rotations.
@export var disable_build_auto_rotate: bool = false
func build_def_text() -> String:
_set_model()
return super()
func _set_model():
if not scene_file:
return
var gltf_state := GLTFState.new()
var path = trenchbroom_game_dir + "/" + trenchbroom_models_dir + "/" + classname + ".glb"
if not _create_gltf_file(gltf_state, path):
printerr("could not create gltf file")
return
const model_key := "model"
const size_key := "size"
path = path.replace(trenchbroom_game_dir + "/", "")
var model_value: String = "{\"path\": \"%s\", \"scale\": %s}" % [path, scale_expression]
meta_properties[model_key] = model_value
if enable_aabb_size_calc:
meta_properties[size_key] = _get_bounding_box(gltf_state.meshes)
func _create_gltf_file(gltf_state: GLTFState, path: String) -> bool:
var error := 0
var gltf_document := GLTFDocument.new()
gltf_state.create_animations = false
var node = scene_file.instantiate()
if not node is Node3D:
printerr("Scene does not inherit Node3D")
node.queue_free()
return false
(node as Node3D).rotate_y(-PI * 0.5)
gltf_document.append_from_scene(node, gltf_state)
if error != OK:
printerr("Failed appending to gltf document", error)
node.queue_free()
return false
call_deferred("_save_to_file_system", gltf_document, gltf_state, path)
node.call_deferred("queue_free")
return true
func _save_to_file_system(gltf_document: GLTFDocument, gltf_state: GLTFState, export_path: String):
var error := 0
error = DirAccess.make_dir_recursive_absolute(export_path.get_base_dir())
if error != OK:
printerr("Failed creating dir", error)
return
error = gltf_document.write_to_filesystem(gltf_state, export_path)
if error != OK:
printerr("Failed writing to file system", error)
return
print('exported model ', export_path)
func _get_bounding_box(meshes: Array[GLTFMesh]) -> AABB:
var aabb := AABB()
for mesh in meshes:
aabb.merge(mesh.mesh.get_mesh().get_aabb())
return aabb |
|
Meanwhile this can go in at line 469 or whatever in if entity_definition is QodotFGDModelPointClass and 'rotation_degrees' in node:
if !(entity_definition as QodotFGDModelPointClass).disable_build_auto_rotate:
var angles: Vector3 = Vector3(0, 180, 0)
if 'angles' in properties or 'mangle' in properties:
var key := 'angles' if 'angles' in properties else 'mangle'
if properties[key] is Vector3:
angles = properties[key]
elif properties[key] is String:
var arr: Array[String] = (properties["mangle"] as String).split(" ")
for i in maxi(arr.size(), 3):
angles[i] = arr[i].to_float()
if classname.find("light"):
angles = Vector3(angles.y, angles.x + 180.0, -angles.z) # lights
elif classname == "info_intermission":
angles = Vector3(-angles.x, angles.y + 180.0, -angles.z) # info_intermission
else:
angles = Vector3(angles.x, angles.y + 180.0, -angles.z) # default
elif 'angle' in properties:
angles = Vector3(0.0, (properties['angle'] as float) + 180.0, 0.0)
node.rotation_degrees = angles |
With my solution, that's not really the goal. Why would the TB models be committed to source control if they are generated when the FGD is exported? This saves some storage usage in git, which might be desired. I've also added a I'll edit some code to make the change even more radical, but doing so allows Qodot to automate things while allowing the user to override settings if desired. I don't understand why you apply a rotation of 180 degrees to the y axis? This makes the model point the wrong way for me. |
|
I've added a couple of changes that I initially wanted to avoid but as I want to streamline Qodot I decided I'll just do it all my way and gather feedback afterwards.. Anyway: I've added an Options class that is passed along when the class_name QodotBuildDefTextOptions
var trenchbroom_project_dir: String
var model_export_dir: String
var create_ignore_files: bool
var scale: intThis contains settings defined in the game config resource and passes them along to the Including that, there's also a new category in the @export_category("Model Exporter")
## The "Game Path" that is defined in the Trenchbroom game settings
## Will use the Godot project if left empty
@export_global_dir var trenchbroom_project_dir := ""
## The path relative to the Trenchbroom game folder where models are exported to
@export var trenchbroom_exported_model_dir := ""
## Creates a .gdignore and .gitignore file when models exported so they're ignored
## by Godot and source control
@export var create_ignore_files_on_export := trueThese are defaults, for when the |
gwenvis
changed the title
Because if you're working with a team, they aren't always exporting the FGD themselves. Sometimes they're just copying the FGD changes to their folder. Other times there might not be changes to the FGD, just adjustments to the model, so all you want to do is have those changes copied over. Maybe you're working on the project across several machines. In any case, the goal of this resource is trying to reduce steps. It doesn't do anything that can't already be done.
I think I found a big hitch in your plan then. The 180 degree rotation is because -Z is forward in Godot. When you model with the default axes in Blender (+Y forward) then you end up with a model that faces +Z on import into Godot. If you're just having the model be a static model, no movement, then you'll be fine with your original rotation setup I'm sure. But if you child your model to a Node3D and need to apply transforms that depend upon -Z forward (say a CharacterBody3D) then you'll have a model that's always facing backwards unless you pre-apply a rotation of 180 to the model. The formula I made accounts for this. I also apply a -90 degrees rotation to the final display GLTF output so that the model points in the correct direction in TB (facing the direction of the little point entity arrow). You could add a
Dunno why you did that. You can just do all of that through the
Once again, not actually needed and also out of scope for GameConfig. GameConfig doesn't control model directory locations, the FGD file's individual entity entries do. What happens when you want to organize your display models into a different directory structure? I'd say don't touch GameConfig at all with this class. I would say it's also out of scope for the FGD file too, since it's also not an FGD wide setting. It's best left as an option on the entity resource itself, since it's specific to the entity in the FGD file. I think you're overdesigning the resource. All of the things you want to do I already implemented in my changes to your script, with the exception of the I don't think you should auto-add the It's important to remember that each TB file and each resource has their own special distinct roles and responsibilities. Qodot's job is to interpret those responsibilities, not redefine them. Something I keep in mind with all of my own contributions to the plugin. |
I'm just thinking about working on this project with a team, which is what I want to use it for. Needing to set the trenchbroom game dir in every entity resource means that whenever someone wants to export entities themselves they have to go around every resource and change the path to match theirs. If you're working on this alone, that's fine, you can just duplicate the resource and call it a day. But work with more people or 2 different system? Back to changing every path to match your system's file structure again whenever you create a new entity or want to work on maps. With a little more handholding it could make life much easier for developers. Why should it be hard? I'm actually thinking it would be better to have the Trenchbroom project folder be an editor setting for this reason, then I could avoid touching
Oh I see, you rotate the model by -90 degrees. I opted for positive 90 degrees because then the model matches the same rotation in both Trenchbroom and Blender and I don't have to rotate later. I ignored the little arrow because I assumed it did not matter as you can see the visual representation of the model anyway. You'd prefer your method of rotating it by -90 when exporting and then 180 when importing? I'll go ahead and remove the options class I made and instead put it into an EditorSetting. I just do not agree with making life harder and demanding the paths have to be set for every entity resource. Source control just makes global directories very hard to use. |
|
Alright, to avoid creating a class that is passed around to different objects I've decided to instead make it an editor setting, which is imo a much better solution in the case of dealing with source control. Adding an editor setting however means that every project uses the same setting and that can also be quite annoying. So what I've actually done now is add project specific settings! There's now a resource where you can define your Trenchbroom 'games/' path and also the project path which is then used for exporting the object and looking up the This in turn also adds #82 because I've done the work for that already- with the added benefit of it being per-project based. |
That little arrow is what tells you which direction is forward! Don't ignore it! But yes, rotating it -90 degrees gives it the correct forward. Whenever you import a model into Godot it always faces backwards by default.
And I just do not agree that you should be mixing and muddying roles and responsibilities of the various class types that don't actually control those settings in the implementation.
I just read through the code, and this I do like. It's handy and non-invasive. I would've liked this as a separate pull request personally, just because it looks clean, doesn't conflict with or change anything else, just adds a new requested feature safely. I know you need it for what you're trying to do, but it feels like it should've been its own pull because then it'd probably get merged more quickly even while you're still baking this addon. I also like this implementation more than #82 because you follow the same design pattern as every other class by prioritizing GameConfig's definitions over the Editor Setting's. Maybe you could split this off of this PR and create a new PR for it? Looking through Despite the madness that is this PR's changelog, if I understand everything correctly it looks like the current implementation does this:
Theo only files that are added are:
The only files that should end up modified with small non-breaking changes are:
If my interpretation of the current design intention is correct, then I think that assuages all of my previous concerns and looks good to me. It allows users to keep working as they did before while also adding a couple features that should be useful for most of our community. The only big problem I see is that right now it looks like your current fork is significantly behind. I'd recommend trying to sync your forks, just to be safe with all the new additions. QodotMap, SolidClass, and GameConfig had some significant features added to them and it looks like your PR is about to undo all of them. |
Ah yikes, thanks for noticing that. I've accidentally overwritten the files after merging. |
addons/qodot/src/resources/game-definitions/fgd/qodot_fgd_model_point_class.gd
Show resolved
Hide resolved
addons/qodot/src/resources/game-definitions/fgd/qodot_fgd_model_point_class.gd
Outdated
Show resolved
Hide resolved
|
Other than those 2 comments, everything looks good to me. I'll give it a test tomorrow, but so far I think it looks good to go. Particularly interested in the Editor Settings addition. |
|
So I tried testing it out. First thing I did was attempt to make a game_editor_settings resource, and was bombarded with errors. I suspect this has to do with it not knowing how to handle empty directory properties. I also looked into the EditorSettings and ProjectSettings classes in Godot, and also how Qodot already handles a different type of ProjectSetting, and I'm going to look into a different approach for it that is more in line with pre-existing Godot and Qodot design patterns. It will still have a Trenchbroom Games Directory and Trenchbroom Game Working Directory though, so your Model Point Class will still be able to hook into that. It will also negate the need for a separate resource to add. I'll also use the GameConfig design pattern you used here. When I'm done with that addition, the only modification required by this PR should be the qodot_map.gd modification. I'll look at getting the ProjectSettings proposal done sometime tonight. |
Can you post what errors you had? It works fine on my end, so I'm not sure what the issue is. I've deployed a potential fix, can you let me know if it works?
I also looked at them both, but don't think either of them are a good solution. EditorSettings means that all values are the same in every project, which is not desirable. ProjectSettings are pushed to source control, which is what I wanted to avoid with the game_editor_settings. Editor Settings are the best options, but then you'd have to change it every time you switch projects. Also: You don't need to create your own I've considered another method; instead of requiring to use the |
The fix worked! I do get one error message now: Yeah, I've been doing research all morning on it and I'm inclined to agree at this point. EditorSettings seem very poorly tutorialized, very messy, very obnoxious to clean up. ProjectSettings don't really solve the source control issues. At best you can make an argument of "just put in res:// manually" for setting the working directory, but it's probably better off sticking close with other editor settings.
Ah, okay. Forgot about that. Useful and clever workaround. In any case, after looking into it I'm inclined to agree with your approach over the others. I'd say anything that breaks existing functionality for GameConfig is a no-go at this point, so I'd have to agree you shouldn't remove those properties. Besides, it's helpful to have the option of per resource overrides. Functionally speaking, I think I finally like the PR lol. That said, there are a lot of naming convention changes I think should be made for both parity and clarity, but also looking deeper at the QodotEntitySettings class, it looks a little obnoxious to add new settings to, so I'm gonna play around with it to see if I can find a more streamlined implementation. Currently looking at enums and a settings array instead of a dictionary, which will allow for more flexibility down the line I think. I'll reply with my results once I finish my changes so you can see if they look good to you, too. Figure that'd be a better way than just pinging you 5000 times for every little review comment. |
|
Alright, so I made a number of changes and refactorings. QodotProjectConfig ClassI've renamed Properties are no longer defined across multiple locations. Instead all properties are defined at the top of the script, allowing for easier extensibility and clearer functionality. I've also removed the property specific functions and changed the identifier argument in I also added an This code replaces the entire previous class script. @tool
class_name QodotProjectConfig
extends Resource
enum {
QODOT_TB_GAMES_FOLDER,
QODOT_TB_WORKING_FOLDER,
QODOT_TB_MODELS_FOLDER
}
const CONFIG_PROPERTIES: Array[Dictionary] = [
{
"name": "trenchbroom_games_folder",
"usage": PROPERTY_USAGE_EDITOR,
"type": TYPE_STRING,
"hint": PROPERTY_HINT_GLOBAL_DIR
},
{
"name": "trenchbroom_working_folder",
"usage": PROPERTY_USAGE_EDITOR,
"type": TYPE_STRING,
"hint": PROPERTY_HINT_GLOBAL_DIR
},
{
"name": "trenchbroom_models_folder",
"usage": PROPERTY_USAGE_EDITOR,
"type": TYPE_STRING
}
]
@export var export_project_config: bool:
get:
return export_project_config
set(value):
_save_settings()
export_project_config = false
var trenchbroom_games_folder: String:
get:
_try_loading()
return settings_dict.get(CONFIG_PROPERTIES[QODOT_TB_GAMES_FOLDER]["name"], '')
set(value):
settings_dict[CONFIG_PROPERTIES[QODOT_TB_GAMES_FOLDER]["name"]] = value
#_save_settings()
var trenchbroom_working_folder: String:
get:
_try_loading()
return settings_dict.get(CONFIG_PROPERTIES[QODOT_TB_WORKING_FOLDER]["name"], '')
set(value):
settings_dict[CONFIG_PROPERTIES[QODOT_TB_WORKING_FOLDER]["name"]] = value
#_save_settings()
var trenchbroom_models_folder: String:
get:
_try_loading()
return settings_dict.get(CONFIG_PROPERTIES[QODOT_TB_MODELS_FOLDER]["name"], '')
set(value):
settings_dict[CONFIG_PROPERTIES[QODOT_TB_MODELS_FOLDER]["name"]] = value
#_save_settings()
var settings_dict: Dictionary
var loaded := false
static func get_setting(setting: int, type: int = TYPE_STRING) -> Variant:
var settings = load("res://addons/qodot/qodot_project_config.tres")
if not settings.loaded: settings._load_settings()
var value = settings.settings_dict.get(CONFIG_PROPERTIES[setting]["name"], null)
match type:
TYPE_STRING: return value as String
TYPE_INT: return value as int
TYPE_FLOAT: return value as float
TYPE_BOOL: return value as bool
TYPE_ARRAY: return value as Array
TYPE_DICTIONARY: return value as Dictionary
_:
printerr("Invalid setting return type! Returning NULL")
return null
func _get_property_list() -> Array:
return CONFIG_PROPERTIES.duplicate(true)
func _load_settings() -> void:
loaded = true
var path = _get_path()
var settings
if not FileAccess.file_exists(path):
_save_settings()
settings = FileAccess.get_file_as_string(path)
settings_dict = {}
if not settings or settings.is_empty(): return
settings = JSON.parse_string(settings)
for key in settings.keys():
settings_dict[key] = settings[key]
print("Loaded settings from ", path)
notify_property_list_changed()
func _try_loading() -> void:
if not loaded: _load_settings()
func _save_settings() -> void:
for i in CONFIG_PROPERTIES.size():
settings_dict[CONFIG_PROPERTIES[i]["name"]] = get(CONFIG_PROPERTIES[i]["name"])
var path = _get_path()
var file = FileAccess.open(path, FileAccess.WRITE)
var json = JSON.stringify(settings_dict)
file.store_line(json)
loaded = false
print("Saved settings to ", path)
func _get_path() -> String:
var application_name = ProjectSettings.get('application/config/name')
return "user://" + application_name + "QodotConfig.json"QodotProjectConfig ResourceThe [gd_resource type="Resource" script_class="QodotProjectConfig" load_steps=2 format=3 uid="uid://lweelxemje8g"]
[ext_resource type="Script" path="res://addons/qodot/src/util/qodot_project_config.gd" id="1_ftuv0"]
[resource]
script = ExtResource("1_ftuv0")
export_project_config = falseTrenchbroomGameConfigGameConfig has had a slight modification (and a bug fix) as a result. It's still the same single method modification overall though. var folder = trenchbroom_games_folder
if folder.is_empty():
folder = QodotProjectConfig.get_setting(QodotProjectConfig.QODOT_TB_GAMES_FOLDER)
print(folder)
if folder.is_empty():
print("Skipping export: No TrenchBroom games folder")
return
# Create config folder name by combining games folder with the game name as a directory
var config_folder = folder + "/" + game_nameQodotFGDModelPointClassSo the changes for this revolve around the model_export. The variable has been renamed for consistency with QodotProjectConfig. I also removed the default value, since it will be used as an override of the QodotProjectConfig. I think this would be a preferable change to you and a number of others, and it follows with pre-established design patterns. @tool
class_name QodotFGDModelPointClass
extends QodotFGDPointClass
## Optional - if empty, will use the game dir provided when exported.
@export_global_dir var trenchbroom_game_dir = ""
## Display model export folder override
@export var trenchbroom_models_folder := ""func _get_export_dir() -> String:
var tb_game_dir = QodotProjectConfig.get_setting(QodotProjectConfig.QODOT_TB_WORKING_FOLDER) if trenchbroom_game_dir.is_empty() else trenchbroom_game_dir
var export_dir = QodotProjectConfig.get_setting(QodotProjectConfig.QODOT_TB_MODELS_FOLDER) if trenchbroom_models_folder.is_empty() else trenchbroom_models_folder
return tb_game_dir.path_join(export_dir).path_join('%s.glb' % classname)
func _get_local_path() -> String:
var export_dir = QodotProjectConfig.get_setting(QodotProjectConfig.QODOT_TB_MODELS_FOLDER) if trenchbroom_models_folder.is_empty() else trenchbroom_models_folder
return export_dir.path_join('%s.glb' % classname)QodotMapNo additional changes to QodotMap, everything you have in your latest version looks good to me. I've already tested this pretty extensively, and it seems to work quite well. If you have no objections or questions to these modifications and want to merge them to your PR so I can test the PR again for a final review then we can finally see about getting this addition merged to main. |
|
Wow you work fast, I was just typing a message and then your refactor shows up :p. I like what you've done. If you want, I could make the project settings even more streamlined. (ie. to add a new setting you just add a new enum). Other than that though, I'm satisfied. I'll finish this up tomorrow as it's rather late here now and then hopefully someone else can review this PR as well :). |
I think it's probably better to keep the CONFIG_PROPERTIES Array, since not every property is guaranteed to be a String type and the extra flexibility is good to have. I pinged the Embers on the Discord, so hopefully they can check this out and give their thoughts if they have any before we go ahead and merge it. |
|
There we go! I applied your changes and I've made it a little easier to add new properties. All you have to do now is add a new enum field and use |
|
I'm gonna test out the latest merge here and report back. I can't remember if the I also think QodotProjectConfig won't return Vectors and AABBs from the config file properly since we don't have a real handler for it atm but I'm not as concerned with that just yet since we don't have any config properties that require them and it can be easily added without breaking projects later. |
|
I've added you to a collaborator of my fork, so if you find out stuff you still want to change, feel free to do so. If you'd rather divert it to me, just post what changes you want and I'll apply them as soon as possible :p. |
A little code and comment cleanup. Renamed certain vars and methods for parity and clarity. Updated the size generation.
Changed `apply_rotation_on_import` to `apply_rotation_on_map_build`
Removed NONE value from PROPERTY enum. Removed duplication of CONFIG_PROPERTY in the _get_config_property() method, since we were only reading from a sub-dictionary in the array.
|
Alright, I think those are all the changes I wanted to see. If you want to review it one more time make sure everything looks good, then I can see about finally merging it with the main branch. EDIT: Thinking it might be worth it to move the @EMBYRDEV @DeerTears would this be something you think would be okay? It would technically be a "breaking" change, since it might break the rotation code other users have come up with, so they'd either have to toggle it off on their entity defs or erase there rotation code... but I think it might be worth it. I know we could always default it to off but I feel as though there's more benefit to having it on by default. I'll follow your leads though. |
|
Testing now! If I have any questions as I go through I'll post them here, but let me know if discord works too just for the immediacy ^ ^; |
Moving auto rotation control to Point Entities from Model Point Entities since it's a base property of all point entities in Trenchbroom.
Rotation is a property of all point entities in Trenchbroom, not just those with models. Makes more sense to provide auto rotation control to Point Entity, which Model Point inherits from.
Rotations are a property of all point entities in Trenchbroom, so it makes more sense to add the auto rotation control to all Point Entites rather than just Model Point Entities.
`angles_raw` now checks to make sure it has 3 elements before trying to apply, otherwise throw an error. In case we get invalid `angles` or `mangle` key values. `mangle` handles lights and info_intermission differently than `angles` in Trenchbroom, so added a check for that. Now apply angles_raw values earlier.
|
@gwenvis Alright, I'm 98% positive I've finally made all the tweaks I think were left. I've changed the optional auto-rotation to all Point Entities instead of just the Model Point Class, since it seems more useful as a universal point entity option since not all point entities have models but Trenchbroom handles rotations for all point entities. @DeerTears @EMBYRDEV I think this is the final version of this. It does modify some pretty big things, so I wanted to make sure this PR was cool with you two before I go ahead and merge it to main. |
Normal rotations and info_intermission rotations were mixed up.
Doesn't seem to actually revert the property on export, so removing that bit of the description. May or may not try to add this feature later.
I've added a seperate entity definition called a
model-point-classthat exports the given scene as a.gltffile in the export folder and then adds a meta-property pointing to that file. The model is automatically scaled 16x (customisable).When Qodot builds the map it automatically fixes the rotations of the entity so it properly reflects what it looks like in Trenchbroom.
I'm uncertain whether this addition should be merged into the main project. The
model-pointclass I've introduced could potentially serve as a valuable community resource, though I believe it's very useful and could be considered a core component to Qodot.If incorporating this feature into Qodot isn't necessary, I'd like to use this PR to seek guidance on an alternative approach for applying rotation to objects without modifying the
qodot_map.gdclass. One possible solution is inheriting from thegodot_entity.gdclass and applying the rotation when the properties are applied, but this would require users to attach the script to each entity individually, which might not be the most intuitive solution.