search

godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.05k stars 21.18k forks source link

Default material import settings don't work as expected #11452

Closed NathanWarden closed 6 years ago

NathanWarden commented 7 years ago

When importing a model, the default import settings for it's materials are set to "Location: Mesh" and "Storage: Files", however the materials used are "Built-In" instead of using the generated "Files".

Edit: This is using the head commit of Godot 3 as of the writing of this issue.

Steps to reproduce: Material_Import_Issue.zip

Files supplied are for your convenience to skip steps 1 and 2 below.

Location: Mesh (Default, doesn't work as expected)

1) Create a model (IE. a cube in Blender) 2) Export it to dae format using the Better Collada Exporter into your Godot project. 3) Notice materials are generated into the folder alongside the dae file.

Location: Node (Works as expected)

Do all of the above steps, except: 1) Create a model (IE. a cube in Blender) 2) Export it to dae format using the Better Collada Exporter into your Godot project. 3) Materials are generated into the folder alongside the dae file, as expected. 4) Change the import setting for Materials>Location to "Node" 5) Add it as an instanced scene into another scene. 6) Set it as having editable children 7) Click on the MeshInstance Node, then under the Materials foldout click the Material 8) Change the material 9) Click save to save the material 10) Notice it saves the material as expected since it's using the "Files" that it generated.

Attached Project Notes

Includes only two Cube.dae files with their associated '.import' files, but one is set to "Mesh" as it's materials location and the other is set to "Node", but no '.import' directory is included so as to cause these dae files to freshly import when you open this project.

Thanks :)

reduz commented 7 years ago

I just fixed it

On Sep 20, 2017 9:08 PM, "Nathan" notifications@github.com wrote:

When importing a model, the default import settings for it's materials are set to "Location: Mesh" and "Storage: Files", however the materials used are "Built-In" instead of using the generated "Files".

Steps to reproduce: Material_Import_Issue.zip https://github.com/godotengine/godot/files/1319567/Material_Import_Issue.zip

Files supplied are for your convenience to skip steps 1 and 2 below.

Location: Mesh (Default, doesn't work as expected)

  1. Create a model (IE. a cube in Blender)
  2. Export it to dae format using the Better Collada Exporter into your Godot project.
  3. Notice materials are generated into the folder alongside the dae file.
    • This is expected since "Files" is the selected storage location.
  4. Add it as an instanced scene into another scene.
  5. Set it as having editable children
  6. Click on the MeshInstance Node, Mesh, then the Material
  7. Change the material
  8. Click save to save the material
  9. It asks to save a new material even though one was imported as a separate resource.
    • In other words, the "Files" storage location import setting isn't causing the mesh to use the stored material, it's using a built-in one instead

Location: Node (Works as expected)

Do all of the above steps, except:

  1. Create a model (IE. a cube in Blender)
  2. Export it to dae format using the Better Collada Exporter into your Godot project.
  3. Materials are generated into the folder alongside the dae file, as expected.
  4. Change the import setting for Materials>Location to "Node"
  5. Add it as an instanced scene into another scene.
  6. Set it as having editable children
  7. Click on the MeshInstance Node, then under the Materials foldout click the Material
  8. Change the material
  9. Click save to save the material
  10. Notice it saves the material as expected since it's using the "Files" that it generated.

Attached Project Notes

Includes only two Cube.dae files with their associated '.import' files, but one is set to "Mesh" as it's materials location and the other is set to "Node", but no '.import' directory is included so as to cause these dae files to freshly import when you open this project.

Thanks :)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/11452, or mute the thread https://github.com/notifications/unsubscribe-auth/AF-Z2wQgeNvvUxgAGeu6oAmcRK9b8OkOks5skajxgaJpZM4Peol7 .

NathanWarden commented 7 years ago

Wow, that was fast :) I'm getting an error on compile for both Windows and Linux now though.

Windows

[ 72%] [94mCompiling [95m==> [93meditor\import\resource_importer_scene.cpp[0m
resource_importer_scene.cpp
editor\import\resource_importer_scene.cpp(257): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(262): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(264): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(269): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(275): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(279): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(298): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(322): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(324): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(358): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(361): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(375): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(395): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(582): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(957): error C2065: 'PRESET_SEPERATE_MATERIALS_AND_ANIMATIONS': undeclared identifier
editor\import\resource_importer_scene.cpp(957): error C2065: 'PRESET_SEPERATE_MESHES_MATERIALS_AND_ANIMATIONS': undeclared identifier
editor\import\resource_importer_scene.cpp(958): error C2065: 'PRESET_SEPERATE_MESHES_MATERIALS_AND_ANIMATIONS': undeclared identifier
editor\import\resource_importer_scene.cpp(960): error C2065: 'PRESET_SEPERATE_ANIMATIONS': undeclared identifier
editor\import\resource_importer_scene.cpp(960): error C2065: 'PRESET_SEPERATE_MATERIALS_AND_ANIMATIONS': undeclared identifier
editor\import\resource_importer_scene.cpp(960): error C2065: 'PRESET_SEPERATE_MESHES_MATERIALS_AND_ANIMATIONS': undeclared identifier
editor\import\resource_importer_scene.cpp(1151): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(1186): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
editor\import\resource_importer_scene.cpp(1193): warning C4291: 'void *operator new(::size_t,const char *)': no matching operator delete found; memory will not be freed if initialization throws an exception
f:\projects\opensource\godot\core\os/memory.h(70): note: see declaration of 'operator new'
scons: *** [editor\import\resource_importer_scene.windows.tools.64.obj] Error 2
scons: building terminated because of errors.

Ubuntu

editor/import/resource_importer_scene.cpp: In member function 'virtual void ResourceImporterScene::get_import_options(List<ResourceImporter::ImportOption>*, int) const':
editor/import/resource_importer_scene.cpp:957:180: error: 'PRESET_SEPERATE_MATERIALS_AND_ANIMATIONS' was not declared in this scope
  bool materials_out = p_preset == PRESET_SEPARATE_MATERIALS || p_preset == PRESET_SEPARATE_MESHES_AND_MATERIALS || p_preset == PRESET_MULTIPLE_SCENES_AND_MATERIALS || p_preset == PRESET_SEPERATE_MATERIALS_AND_ANIMATIONS || p_pre
set == PRESET_SEPERATE_MESHES_MATERIALS_AND_ANIMATIONS;
                                                                                                                                                                                    ^
editor/import/resource_importer_scene.cpp:957:236: error: 'PRESET_SEPERATE_MESHES_MATERIALS_AND_ANIMATIONS' was not declared in this scope
  bool materials_out = p_preset == PRESET_SEPARATE_MATERIALS || p_preset == PRESET_SEPARATE_MESHES_AND_MATERIALS || p_preset == PRESET_MULTIPLE_SCENES_AND_MATERIALS || p_preset == PRESET_SEPERATE_MATERIALS_AND_ANIMATIONS || p_pre
set == PRESET_SEPERATE_MESHES_MATERIALS_AND_ANIMATIONS;

       ^
editor/import/resource_importer_scene.cpp:960:36: error: 'PRESET_SEPERATE_ANIMATIONS' was not declared in this scope
  bool animations_out = p_preset == PRESET_SEPERATE_ANIMATIONS || p_preset == PRESET_SEPARATE_MESHES_AND_ANIMATIONS || p_preset == PRESET_SEPERATE_MATERIALS_AND_ANIMATIONS || p_preset == PRESET_SEPERATE_MESHES_MATERIALS_AND_ANIMA
TIONS;
                                    ^
scons: *** [editor/import/resource_importer_scene.windows.tools.32.o] Error 1
scons: building terminated because of errors.
NathanWarden commented 7 years ago

Hi Juan, it looks like that commit was going in the right direction, but doesn't quite fix the issue. Here are two screen recordings showing the material issue and also that it has the same kind of issue with the imported Mesh resource.

Thanks!

Import_Issue_Videos.zip

reduz commented 7 years ago

Import code for scenes is here: https://github.com/godotengine/godot/blob/master/editor/import/resource_importer_scene.cpp

it exports many presets and options and then it uses them. For Materials it can keep the previous ones with one of the options.

NathanWarden commented 7 years ago

@reduz Is this a hint for me to try and fix it? I'd be happy to start taking a look either tonight or tomorrow morning :)

reduz commented 7 years ago

It's for the hero wanted campaign :) I am describing a bit how fixing of issues could be done.. will hopefully launch it tomorrow

On Nov 6, 2017 8:26 PM, "Nathan" notifications@github.com wrote:

@reduz https://github.com/reduz Is this a hint for me to try and fix it? I'd be happy to start taking a look either tonight or tomorrow morning :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/11452#issuecomment-342322715, or mute the thread https://github.com/notifications/unsubscribe-auth/AF-Z22RqKAH9AxAdW1PsZVe-YJyr43Xoks5sz5W0gaJpZM4Peol7 .

NathanWarden commented 7 years ago

Haha, cool, I'll take a look tonight then. Thanks for pointing to the right file as just finding the right starting place tends to be what takes half the time for me. If I run into a brick wall I'll let you know.

NathanWarden commented 7 years ago

@reduz Okay Juan, I think I got it. It was assigning the in memory version of the material (and meshes) on first import instead of the on disk resource. So, now instead of assigning the in memory version that the saved resource is based on, it loads the material/mesh that was just saved back from disk... and everything now seems to work. :)

I also changed the lines:

Ref<Material> existing = ResourceLoader::load(ext_name);
p_materials[mat] = existing;

to

p_materials[mat] = ResourceLoader::load(ext_name);

Let me know if there's a problem with doing this.

Thanks :)

Zireael07 commented 7 years ago

Nathan, I think a certain person from the Discord will be very happy. I will also see if it fixes my issue with mipmap settings not applying on reimport.

NathanWarden commented 7 years ago

@Zireael07 Thanks. I hope it will help some people out. I don't think it will fix mipmap settings though as this fix really only has to do with assignment of materials and meshes on initial import.

I have some time this morning, Is there a bug filed for your issue with a reproducible project?

Zireael07 commented 7 years ago

I filed a bug at #10221. I don't have a reproducible project but it's fairly easy to reproduce - I could send you a texture file or two that make it easy to see the problem if you slap them on a quad.

NathanWarden commented 7 years ago

@Zireael07 I commented on your bug report. Having a texture that causes the issue would be very helpful. Thanks :)