Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add entity properties #69

Closed
wants to merge 8 commits into from

Conversation

rand0m-cloud
Copy link

Sorry for intruding on #66, this is better suited as a PR to be considered after #66 is merged.
This work is based off of #66's last commit.

I added a new resource called QodotEntityProperties that allows the user to define the property's name, short/long description, and a default value. The resource is exported through the QodotPointEntityDefinition.

I modified the build_entity_spawns to check for defined properties and apply any defined properties found to the constructed node. It is expected that the user defines variables on their entity's scene and creates a property definition matching it.

The current work only includes integer and string property defining, and needs to be expanded to the available property types. I am unsure how to implement flags or choice types, so I am asking for discussion around it.

Finally, this work has some bugs that I am not sure how to solve.
If I do:

  1. Define an entity with a custom property
  2. Create the entity in TrenchBroom with the property set to a value
  3. Duplicate the entity
  4. Reload the map into Qodot

Only one of the instanced entities has the custom value instead of both entities having the custom value.

@Shfty
Copy link
Member

Shfty commented Feb 8, 2020

Apologies for the slow response. I've pulled down the changes and given them a look over, and I think this is handy functionality to have. However, it's not mergeable as-is. There's an issue with the FGD generation, and I'm not sure about the structure- having one resource file per property seems like overkill.

Instead, I'd expose a Dictionary inside the entity definition itself and take advantage of the inspector UI's ability to define types for each entry. That way, if dictionary['property'] is string: and the like could be used to determine the format during export.

Flags is a tricky one, since it's ultimately exposed as a bitmask. I'd implement that as a Dictionary of bools- that way each flag can have a name stored in its key, and the attached booleans can be used to determine defaults (if default flags are supported by the FGD format, i'm not sure offhand).

I'd probably implement choices as a Dictionary as well, since that allows a key and value to be specified for each choice. It does introduce some complexity in terms of identifying whether a dictionary contains flags or choices- that might be solvable by wrapping each one in its own custom class, but it might become cumbersome depending on that state of inspector property exports in 3.2.

Going back to the issue with FGD generation, it looks like default values for string properties are broken. The generated FGD doesn't have double-quotes around them, which causes TrenchBroom to error out during parse. Adding them manually makes it work as expected.

@Shfty
Copy link
Member

Shfty commented Feb 8, 2020

I went ahead and refactored the existing FGD export support to include BaseClass and SolidClass as well as PointClass, and setup more flexible systems for defining class metaproperties, properties, descriptions, and so on.

The implementation is largely as mentioned in my previous comment, though I ended up separating choices and flags into dictionaries and nested arrays respectively since there's no custom class support in the dictionary inspector. Flags are a tad clunky, but they work and get all the data across as needed.

Next up is hooking up a flexible map file > entity workflow, which this pull partially covers. I'll take a look into this over the weekend.

@rand0m-cloud
Copy link
Author

I've taken a look at the latest commits and I like your implementation over mine. You can close the pull request unless you want further discussion here.

@Shfty
Copy link
Member

Shfty commented Feb 9, 2020

Alrighty, thanks for the contribution- sometimes it helps to have someone poke me with some code in order to get a new feature off the ground!

Further discussion can go on in #8

@Shfty Shfty closed this Feb 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants