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

Typing, saving as resource, icon and other tweaks #24

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

ckaiser
Copy link
Contributor

@ckaiser ckaiser commented May 24, 2024

Hello!

This PR integrates quite a few changes I made when I started to play around with this addon for a proof of concept I'm building, and it kinda snowballed in size so apologies in advance for the bigness of it all.

A lot of the changes feed into each other, although I could isolate them into separate PRs if some stuff is unwanted/not in scope for the vision of the addon.

Type Hints / Performance

I've added type hints to most (but not all, so that's a WIP) of the code, which has some nice performance implications:
On a big test map with a lot of geometry, displacements and entities, import time has gone from 2300ms to 1500ms.

I've also done some other minor changes for performance, like using groups to find all the VMFNodes, using split_floats, etc

Saving imported geometry as a resource

I've added the option to generate binary resource files when importing geometry and generating collisions, this allows them to be compressed and un-bloats the scene files, which is better for editor load times and generally tidier IMO, plus if any instances share the same vmf file they'll all be updated once a map that uses them is re-imported

VMFNode

Added a "use external file" boolean property, so that you can pick which one you want to use, if an internal resource or an external file. This only changes the way the editor file picker works.

The main issue with global file paths is that it makes collaboration harder, since each person might have map files stored in different places.

I've also added a "Resource Generation" group which can toggle on and off the resource file generation, by default it's on but it's reasonable to might want to disable it for smaller things.

Node Icon

The icon was basically invisible on light modes so I took the liberty of making a very simple one, this one also changes colors depending on the editor theme.

Other stuff

I've also done some general cleanup/tweaks, I think in general this might need some testing, especially with a bigger project with more nodes and interconnected parts, I haven't gotten around to downloading your sixside project to test it with that, but that'd be a smart thing to do.

I tried to keep the code style the same as the rest of the project, but lmk if any changes are needed. Cheers!

@H2xDev
Copy link
Owner

H2xDev commented May 24, 2024

Hello! Thank you for your work.

Added a "use external file" boolean property, so that you can pick which one you want to use, if an internal resource or an external file. This only changes the way the editor file picker works.

I think it should be documented in the docs. Could you do it here?

The new node icon looks good. I like it.

Testing

Of course I'll try to test it. You can also test the plugin via importing the Half-life 2/TF2/Portal maps to check the stability. That's how i test it besides the sixside project.

  1. Install HL2/Portal/TF2
  2. Extract all materials from VPKs
  3. Decompile BSPs
  4. Create blank Godot project
  5. Install plugin and import VMFs

Yea, i think i should add it into contribution instruction :P

Codestyle

About the codestyle. There's no any codestyle since i made this project during learning the engine, so i think we should align the codestyle somehow :D

And again. Thank you for you work.

@ckaiser
Copy link
Contributor Author

ckaiser commented May 24, 2024

Oh yeah, documentation, that thing I always forget to do. Alright I'll get that done next.

Regarding code style: I'm more partial to using the traditional snake_case that Godot itself uses, but I usually just follow the one the project had in the first place (and it'd take a while to redo everything so that's kind of a pain)

@ckaiser
Copy link
Contributor Author

ckaiser commented May 25, 2024

I've pushed some documentation, and I tested it by importing one of the Episode 1 maps (didn't bother importing the materials, just meant to the test the geometry for the most part since that's the part that could break in unexpected ways, and the resulting geometry has the same vertex count and appears identical 😌. It's also a much nicer test case for optimization, went from 3450 to 3050 ms so plenty of room for improvement.

@H2xDev
Copy link
Owner

H2xDev commented May 25, 2024

That's good! Thank you.

About the global paths

The main issue with global file paths is that it makes collaboration harder, since each person might have map files stored in different places.

The idea of the vmf.config.json is that every developer will have his own config and the config itself should be in .gitignore. I need to describe the development pipeline well with this tool...

@ckaiser
Copy link
Contributor Author

ckaiser commented May 26, 2024

Yeah, I'm running into the issue that the instance manager was unable to find the instances since I was storing stuff in my Godot folder as opposed to the mod's mapsrc folders, gonna see about adding a fix for that.

Ideally, it should be possible to support both methods, I think storing everything in one place, except for stuff like valve format materials and models which should go in the mod folder is, at least to me, the most comfortable way to set things up.

Something that we could do for "global" files is that instead of saving the absolute path in the VMFNode, we could save a path that is relative to the vmf.config.json, so that way a potential collaborator would only have to point their config file to the mod folder and not have it in the same drive/folder layout

@H2xDev
Copy link
Owner

H2xDev commented May 28, 2024

@ckaiser Sorry for late answer. Yea. You can try to implement it if you didn't yet :)

@H2xDev
Copy link
Owner

H2xDev commented Jun 7, 2024

@ckaiser I would like to merge these changes. I would glad if you resolve the conflicts :)

@ckaiser ckaiser force-pushed the feature/resources-and-typing branch from 6397cfd to b590090 Compare June 7, 2024 15:22
@ckaiser
Copy link
Contributor Author

ckaiser commented Jun 7, 2024

@ckaiser I would like to merge these changes. I would glad if you resolve the conflicts :)

Rebased all the changes on top of develop, don't have the time to fully test it now but it looks alright from a cursory glance

@H2xDev
Copy link
Owner

H2xDev commented Jun 7, 2024

@ckaiser No problem. I'll try to test. Thank you for contribution :)

@H2xDev H2xDev merged commit dd7d0c4 into H2xDev:develop Jun 7, 2024
@H2xDev H2xDev mentioned this pull request Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants