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

3.4.4 Inconsistent getting of Mesh and MeshInstance surface material #61788

Closed
PerplexedPeach opened this issue Jun 7, 2022 · 3 comments
Closed

Comments

@PerplexedPeach
Copy link

Godot version

3.4.4.stable

System information

Windows 10, GLES3, NVIDIA RTX 2060, driver 512.95

Issue description

I have a MeshInstance body that I'd like to retrieve its ShaderMaterial from. Calling body.get_surface_material(1) produces a null instance while calling body.mesh.surface_get_material(1) produces the ShaderMaterial as expected.

In this case, the MeshInstance has no material defined. As I understand, MeshInstances are allowed materials to override its Mesh materials. body.get_active_material works as intended, so that leads me to suspect that get_surface_material works as intended and this is not a bug. However, in that case the documentation is very misleading:

image

This suggests to me that it's actually returning the Material of its Mesh resource (literally what it says), but it does not do that. It instead returns the overriding material this instance is using over that of its Mesh resource, even when it doesn't exist. I suggest a couple of changes:

  • make the documentation clearer that get_surface_material returns the MeshInstance's Material, not the underlying Mesh's, and describe get_active_material and mesh.surface_get_material as alternatives
  • make the behavior when MeshInstance's Material is null to take on the Mesh's material (this might be a breaking change, so probably just go with point 1)

Steps to reproduce

Load a 3D model with at least a single part and material

Minimal reproduction project

No response

@clayjohn
Copy link
Member

clayjohn commented Jun 7, 2022

This was fixed for 4.0 by renaming MeshInstance's get_surface_material() to get_surface_override_material() https://docs.godotengine.org/en/latest/classes/class_meshinstance3d.html#class-meshinstance3d-method-get-surface-override-material

For 3.x we should update the documentation to say that get_surface_material() returns the override material specified in the MeshInstance rather than the material specified on the Mesh

make the behavior when MeshInstance's Material is null to take on the Mesh's material (this might be a breaking change, so probably just go with point 1)

This indeed would be a breaking change and essentially duplicates what get_active_material() already does better. I would stick with a documentation update

@PerplexedPeach
Copy link
Author

Ah I see, just found #479 ; yeah would appreciate the documentation band-aid fix for 3.x

@akien-mga
Copy link
Member

Fixed by #62347.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants