WIP t6 custom map loading#539
Conversation
note: techniques should be dumped in seperate files from technique sets, but compiling them all into the techset works as well but is harder to read.
…instead of a stream.
…eam, and added techset loading code
…missing image check to the BSP compiler to reduce the output prints when an image is missing.
Laupetin
left a comment
There was a problem hiding this comment.
Hey there, thank you for the PR, the feature is pretty awesome :)
I've looked through a lot of the code by now, sorry it takes so long, it's a big PR 😅
I probably won't have that much time in the upcoming days, so i'll submit the comments I have currently.
I haven't looked at CustomMapLinker.h yet, though since I do not know that much about the BSPs yet, I probably won't be able to say about that part 😅
So generally my comments are more related to code structure and memory management for example.
Also please forgive that I'm adding a lot of comments, it's just my style of doing code review (I'm adding a comment when I have any special thoughts about some place so it ends up being pretty verbose) and does not mean that necessarly everything needs to be addressed or must be solved in a particular way necessarly.
Also I can address a lot of the stuff as well, you don't have to do everything 😛
Again, thanks for putting the work in :D Looking forward to this feature and how it evolves
Done: - Moved custom map structures to their own objcommon header file - Updated GfxLightGridRow struct - Reverted shader_bin file path - Renamed Project Creator to BSP Creator - Removed model loading from BSP creator - Cleaned up BSP Creator and updated the names of structs WIP: - Update BSP calculation code to be more readable and use unique/shared ptrs
…imposed limits, beautified code, added comments where logic wasn't very obvious.
|
Right now i'm moving to using GLTF as i found it to not have these issues that FBX has had, hopefully switching over would fix the 1st issue (the branch is updated to use gltf ##now). I agree a warning would be good when it sees not a world material, but when custom materials become possible i doubt many map makers would use the same wpc/wc/etc naming scheme so a full error would be too much. Maybe there's a way to tell if the material will/won't render apart from their naming scheme? Feel free to add a pull request to the branch if you want to add changes though. |
Notes: