H2xDev / GodotVMF

A VMF importer for Godot Engine
MIT License
84 stars 5 forks source link

Displacement feature is incomplete and questionable design choices #18

Closed Lachrymogenic closed 4 months ago

Lachrymogenic commented 4 months ago

First and foremost

I'm somewhat of an experienced developer, so I thought about fixing stuff myself, but I'm not too confident in knowing how GodotVMF works yet and I don't feel like making commits to the project if my use case is going to be extremely specific, so I thought I'd let you know about these kinds of things, because for the past few days, I've been trying to get a custom map, particularly a surf map, to load for testing by using GodotVMF.

Before update 1.2.0 (Displacement) I was able to get the map to load correctly, although the models were missing their textures and everything else was too because I hadn't extracted the vpk. A full map and no textures is completely fine because I can just substitute the missing textures with my own materials, but as of right now, importing the map doesn't seem to work.

First, though, I need to talk about my setup, here is my config:

My config

{
    "entitiesFolder": "res://assets/entities",
    "gameInfoPath": "/home/lachrymogenic/Development/Godot Engine/VMFTest/mesa/",
    "instancesFolder": "res://assets/materials",
    "materialsFolder": "res://assets/materials",
    "mdl2obj": "/home/lachrymogenic/Development/Godot Engine/VMFTest/mdl2obj",
    "modelsFolder": "res://assets/models",
    "nodeConfig": {
        "defaultTextureSize": 512,
        "fallbackMaterial": null,
        "generateCollision": true,
        "generateCollisionForModel": true,
        "ignoreTextures": [
            "TOOLS/TOOLSNODRAW"
        ],
        "importModels": true,
        "importScale": 0.025,
        "overrideModels": true,
        "textureImportMode": 1
    },
    "vtflib": "/home/lachrymogenic/Development/Valve Dev Utilities/VTFCmd/vtfcmd.sh"
}

just incase you're wondering, yes, I put vtflib in a completely different location and yes it is a shell file but it does indeed work, it runs wine and passes all the arguments to the wine executable.

Attempting to study how the code works, I set my gameinfo folder to the folder that BSPSrc produces when you extract a map with the "Extract embedded files" option on. I moved this folder to the project folder. On the old version, it works and imports in custom materials from that map. I also had to change the code because materials and other stuff wouldn't load in general because for some reason the code tries to apply .to_lower() to the path files, for what reason is somewhat unknown. Simply encasing code like

var vtf = (config.gameInfoPath + '/materials/' + baseTexture + '.vtf')\
        .replace('//', '/') \
        .to_lower()

with

var vtf = (config.gameInfoPath) + ('/materials/' + baseTexture + '.vtf')\
        .replace('//', '/') \
        .to_lower()

is what allows me to use my gameinfo path, which includes uppercase letters. I've often had to restart and reload the project multiple times in order to get it working correctly.

Everything was fine in the old version except for an issue where I cannot load custom model materials. But as of right now, my output is spamming this: `Vertex amount (8) must be a multiple of the amount of vertices required by the render primitive (3)' and I'm not sure why though I can only conclude that the Displacement feature is unfinished and other commits have broken functionality of the importer, such as the merging of "develop" into the main branch. This is what it looked like before and after

Before

(I had to redo the before image and models take too much time to load and materials are being a pain rn so I didn't load them in, just pretend that they're there and some textures on the wall are actually there in this image etc.) ramps before

After

after wtf lol godotvmt

With all this being said, I'm not sure if there is anything else that I can provide to help diagnose the issue, maybe I'm the issue, because in trying to revert back to the old version, I have managed to screw myself over and now absolutely NO entities are rendering... except they are now for some reason?

Reverted to commit eedac7d178d02b9b4203babe5b0f082dc43c0fed

I reverted to the version I was using 3 days ago and moved some parts of my config from res://assets (my own folder) to res://examples and renamed the name of the map to use all lower case, turn off import models in the config and loaded up a completely different map and I've managed to get entities to import again

Very weird, I'm going to attempt to study more about how this code works and then modify it to work with my use case, get it to load model materials, but I still thought I should let you know and also that there may be things you might want to tweak, like using all lower case stuff for file paths is a pretty bad idea when most people have capitals in their file paths, I don't know if Windows disregards it or something but Linux for sure doesn't like that.

To be honest, I should have studied it first instead of blindly trying to load in decompiled BSPSrc maps and expecting it to just run perfectly, but if I do manage to learn enough about your project that I'm able to fix some things, I still might be apprehensive to commit due to the potential possibility that whatever works for you breaks for me and vice versa, though I am really keen on using and potentially helping with this project.

To conclude

I don't expect others to do all the work for my issues but what I'm worried about is that the devs of this project have real working model support and displacement support that just works for them but not for me and by optimizing it to work for me, it won't work for them. This might be related to the fact that I am using decompiled maps instead of maps made with Hammer++

H2xDev commented 4 months ago

@Lachrymogenic

Hello! Thank you for test. I already started working on alternative way to import materials without any dependencies like VTFLib (i hope i can do it) and, i guess, the materials problem will be fixed.

Regarding to to_lower. Since VMFs contains all materials in uppercase i should make them to_lower since there might be case-sensitivity OS. In general all developers for Source Engine make names of all assets lowercased. Actually if you have another way to solve this issue - welcome :)

About the speed. I the VMF you loading doesn't contain vertices_plus field for each side then importer will try compute vertices for a brush and if the VMF contains some complex geometry (a sphere for example) it imports much slowly than regular brush. That's why i recommend use Hammer++ since it include precise vertices data into VMF.

About the fix you made with gameInfoPath. Is there a difference (except brackets)?

Also. Could you provide the VMF you tried to import?

Lachrymogenic commented 4 months ago

I already started working on alternative way to import materials without any dependencies like VTFLib (i hope i can do it) and, i guess, the materials problem will be fixed.

Thanks, that would be amazing. For me it's quite a daunting task but your dedication to this is fascinating.

About the fix you made with gameInfoPath. Is there a difference (except brackets)?

gameInfoPath case sensitivity

I'm assuming you mean the .to_lower() thing? Basically, when I encase it in brackets or take out .to_lower() I get these messages:

core/variant/variant_utility.cpp:1111 - [GodotVMF] Missing VMT: /home/lachrymogenic/Development/Godot Engine/VMFTest/mesa/materials/concrete/concretefloor039a.vmt
core/variant/variant_utility.cpp:1111 - [GodotVMF] Missing VMT: /home/lachrymogenic/Development/Godot Engine/VMFTest/mesa/materials/glass/combineglass001a.vmt
core/variant/variant_utility.cpp:1111 - [GodotVMF] Missing VMT: /home/lachrymogenic/Development/Godot Engine/VMFTest/mesa/materials/concrete/concretefloor039a.vmt
core/variant/variant_utility.cpp:1111 - [GodotVMF] Missing VMT: /home/lachrymogenic/Development/Godot Engine/VMFTest/mesa/materials/glass/combineglass001a.vmt

but without encasing them, I get these messages:

core/variant/variant_utility.cpp:1111 - [GodotVMF] Missing VMT: /home/lachrymogenic/development/godot engine/vmftest/mesa/materials/vtextures/neon_b2.vmt
core/variant/variant_utility.cpp:1111 - [GodotVMF] Missing VMT: /home/lachrymogenic/development/godot engine/vmftest/mesa/materials/glass/combineglass001a.vmt
core/variant/variant_utility.cpp:1111 - [GodotVMF] Missing VMT: /home/lachrymogenic/development/godot engine/vmftest/mesa/materials/gm_construct/tanks.vmt
core/variant/variant_utility.cpp:1111 - [GodotVMF] Missing VMT: /home/lachrymogenic/development/godot engine/vmftest/mesa/materials/tools/toolsblack.vmt

What this basically means is that materials will not load because your addons are looking for paths on my system that do not exist, because my paths have upper case letters in them. The one simple fix that I did managed to fix it for me, but for windows, this may be a bad thing? (I don't know) I also had to change my config to not end in a / (Slash)

"gameInfoPath": "/home/lachrymogenic/Development/Godot Engine/VMFTest/mesa"

Otherwise, I'd get messages like

core/variant/variant_utility.cpp:1111 - [GodotVMF] Missing VMT: /home/lachrymogenic/Development/Godot Engine/VMFTest/mesa//materials/tools/toolsblack.vmt

which you'd think the .replace("//","/") would fix, but because I encase (gameInfoPath) that line of code doesn't affect it. Essentially, for me, the best thing to do is to not have this:

Original code (VMTManager.gd)

Static func getTexture

var vtf = (config.gameInfoPath + '/materials/' + baseTexture + '.vtf')\
        .replace('//', '/')\
        .to_lower();

Static func getTextureInfo

var vmtPath = (gameInfoPath + '/materials/' + material + '.vmt').replace('//', '/').to_lower();
var vtf = (gameInfoPath + '/materials/' + baseTexture + '.vtf').replace('//', '/').to_lower();

but instead do this:

My edits

Static func getTexture

var vtf = (config.gameInfoPath) + ('materials/' + baseTexture + '.vtf')\
        .replace('//', '/')\
        .to_lower();

Static func getTextureInfo

var vmtPath = (gameInfoPath) + ('materials/' + material + '.vmt').replace('//', '/').to_lower();
var vtf = (gameInfoPath) + ('materials/' + baseTexture + '.vtf').replace('//', '/').to_lower();

and set my config to "gameInfoPath": "/home/lachrymogenic/Development/Godot Engine/VMFTest/mesa/" WITH the slash at the end, you take off the slash at the end if you do not remove the first slash in /materials/

Demonstration

I would demonstrate that this works, but vtflib is crying at me right now because of the space in "Godot Engine" telling me that "Unknown argument: Engine/VMFTest/mesa/materials/tools/" but because I care, I'll move mesa to my Development folder to demonstrate. Just gotta change my gameInfoPath to "gameInfoPath": "/home/lachrymogenic/Development/mesa/", and... hey presto!

cheese (Please note that they only import materials found inside of the maps custom resources folder and no materials from the actual game, this is intentional because I can easily substitute those textures for my own by creating materials with the same name inside of the materials folder. So when I get an error like "Material TRES not found in project (path)" I can just make a material in that path and the textures will update.)

Must be some sort of argument error, tried running it myself and it thinks the space is a new argument.

Models import too, but their textures seemingly do not.

models

The VMF File

The VMF I tried to import can be acquired by going to https://gamebanana.com/mods/122542 and converting the bsp to vmf by using BSPSrc. Github will not let me upload vmf files, I would upload to pastebin but it is so extremely long and you might need the custom resources for extensive testing.

You can get BSPSrc from here: https://github.com/ata4/bspsrc/releases

Just drag in the surf mesa file, or click add and find it. Make sure you enable this setting to get the folder

bspsrc

H2xDev commented 4 months ago

@Lachrymogenic I made a patch regarding to import geometry. You can pull and try again, it should work.

UPD: Forgot to add your fix regarding to path. I'll do it later.

Lachrymogenic commented 4 months ago

@Lachrymogenic I made a patch regarding to import geometry. You can pull and try again, it should work.

Absolutely! Gonna avoid mistakes like last time and just create a new folder and copy over my config... aaaand.. hey presto ;)

Alas, we are one step closer, my friend! oh yes we are gaming amazing

Absolutely phenomenal work! I'm trying to configure materials right now but apart from that, this is awesome.

Lachrymogenic commented 4 months ago

In regards to materials, it got stuck on creating a few because it was missing a directory, errors so I just made that directory and stuff actually seems to be loading in from the map's custom texture files, which is pretty awesome.

stuff loading in 2

Guessing the rest of it is just textures I have yet to unpack or substitute, but the models use their own custom textures, which doesn't seem to be working. I should probably go unpack game files for testing purposes. There are so many new files inside of my Counter Strike Source installation now, more textures have loaded in, except those rocks.

more textures

white rocks

H2xDev commented 4 months ago

@Lachrymogenic

Looks good! I just pushed native VTF import without VTFCmd. Also fixed import materials for models. It should work now. Switch to feature/VTFTool and give a try :)

Lachrymogenic commented 4 months ago

Sure!

First attempt loading stuff in and all the models are here, with their textures which is sweet. However, nothing else loaded in?

The lava that was there is now not, and no matter what I do, putting the files somewhere else that isn't uppercase or removing .to_lower() they are not loading in, which is strange. There's no error messages or warn messages telling me that VMTs are missing either.

first attempt

I'm not entirely sure why that's happening, even when changing gameInfoPath from the custom folder to an unpacked cstrike folder, no other textures are loading in, only model textures.

Perhaps this is intentional as you are only testing model imports?

H2xDev commented 4 months ago

@Lachrymogenic Hm... I have it work. I should mention that once you changed vmf.config.json you should click on Reload config in VMFNode inspector. Could you try again?

image

Lachrymogenic commented 4 months ago

I tried with this config, I pressed "Reload Config" it's not working:

{
    "entitiesFolder": "res://examples/entities",
    "gameInfoPath": "/home/lachrymogenic/Development/BSPSrc/surf_mesa_d/",
    "instancesFolder": "res://examples/instances",
    "materialsFolder": "res://examples/materials",
    "mdl2obj": "/home/lachrymogenic/Development/Godot Engine/GodotVMF-feature-VTFTool/mdl2obj",
    "modelsFolder": "res://examples/models",
    "nodeConfig": {
        "defaultTextureSize": 512,
        "fallbackMaterial": null,
        "generateCollision": true,
        "generateCollisionForModel": true,
        "ignoreTextures": [
            "TOOLS/TOOLSNODRAW"
        ],
        "importModels": true,
        "importScale": 0.025,
        "overrideModels": true,
        "textureImportMode": 1
    },
}

Still just white. Is there an issue with my config? I can't seem to figure out why it's not working, so I'm going to try and log whether or not it's accessing my surf_mesa_d/materials folder in any way.

H2xDev commented 4 months ago

@Lachrymogenic Set textureImportMode to 2 - Import from mod. Also turn off overrideModels to speed up import

{
    "entitiesFolder": "res://examples/entities",
    "gameInfoPath": "/home/lachrymogenic/Development/BSPSrc/surf_mesa_d/",
    "instancesFolder": "res://examples/instances",
    "materialsFolder": "res://examples/materials",
    "mdl2obj": "/home/lachrymogenic/Development/Godot Engine/GodotVMF-feature-VTFTool/mdl2obj",
    "modelsFolder": "res://examples/models",
    "nodeConfig": {
        "defaultTextureSize": 512,
        "fallbackMaterial": null,
        "generateCollision": true,
        "generateCollisionForModel": true,
        "ignoreTextures": [
            "TOOLS/TOOLSNODRAW"
        ],
        "importModels": true,
        "importScale": 0.025,
-       "overrideModels": true,
+       "overrideModels": false,
-               "textureImportMode": 1
+               "textureImportMode": 2
    },
}
Lachrymogenic commented 4 months ago

Yep, works!

works

Though, I'm not sure if theres a way you can fix this but there might be memory leaks ram usage

This kind of ram usage comes from multiple reimporting though, here is what it is on 1 reimport. godot restarted 1 import

Subsequent imports are showing an increase in ram usage.

5 gigs

H2xDev commented 4 months ago

Yea. Definitely it needs to be fixed. Thank you. I guess i know where the issue is

Lachrymogenic commented 4 months ago

Not sure if it's related to the issue, but I am noticing an increase in error messages like this:

is this related?

errors img 2

name is empty

H2xDev commented 4 months ago

@Lachrymogenic I guess I fixed most of the problems you found. Try again. The branch is the same.

And there are some breaking changes regarding to config. Check the installation docs again :)

Lachrymogenic commented 4 months ago

I've tried it and things are definitely looking a lot better this time around, but I'm not exactly sure what I should be testing for, here. So, I'll try loading and reloading the same map to see if ram is negatively affected or not. After doing so, ram was not affected so much, started at about 4400 MB and is now at 4560 MB with no signs of increasing on further reimports After reimporting one more time it's now 4600 MB, another reimport, still 4600 MB and when importing a different map, 4300 MB.

So definitely no memory leaks this time around, if there are any, then they are very minor.

new mesa

You might find improvements to your project by testing more maps, but that's completely up to you, ofcourse. Other maps, such as, surf_kitsune does sh*t like this:

wtf kitsune

As to whether this is BSPSrc's fault or the mapper's fault, I have no idea (I think it's the mapper's fault), but it makes loading assets for other maps a pain, because you would assume that mappers keep everything all lower cased and for the most part they do, but it seems like not always.

Overall, great update, I'm happy to see that RAM Usage isn't as bad as it was before!

H2xDev commented 4 months ago

That's good! Thank you for test!