KhronosGroup / glTF-CSharp-Loader

C# Reference Loader for glTF
Other
213 stars 60 forks source link

Fixed not being able to assign null to array properties with length restrictions #22

Closed Panzerhandschuh closed 5 years ago

Panzerhandschuh commented 5 years ago

I was running into an issue where I could not remove/nullify extensions from gltf files due to the array length conditions throwing null reference errors.

The conflicting code:

public string[] ExtensionsUsed {
    get {
        return this.m_extensionsUsed;
    }
    set {
        if ((value.Length < 1u)) { // value.Length will through a null reference exception when value is null
            throw new System.ArgumentException("Array not long enough");
        }
        this.m_extensionsUsed = value;
    }
}

Fixed code (after my code gen changes):

public string[] ExtensionsUsed {
    get {
        return this.m_extensionsUsed;
    }
    set {
        if ((value == null)) { // New condition to fix the null reference error
            this.m_extensionsUsed = value;
            return;
        }
        if ((value.Length < 1u)) {
            throw new System.ArgumentException("Array not long enough");
        }
        this.m_extensionsUsed = value;
    }
}

I altered the code gen to assign the property and return if the value is null. I'm not familiar with the code gen system so if there's a better way to apply this fix, let me know.

bghgary commented 5 years ago

Looks good. Do you want an updated nuget package?

Panzerhandschuh commented 5 years ago

No, thanks.