Using `for... in` causes issues with our website

We just started implementing our current babylon project with our actual website and we started getting some unique errors.

specifically this function.

and the reason has to do with this.

and the bug in playground
https://www.babylonjs-playground.com/#JK6DA4

2 Likes

Pinging @nasimiasl as the custom material is a community driven material

1 Like

@Wigen: fancy doing a PR?

1 Like

hi all
thanks for ping me here
as you see it

if ( this._customUniform[ind].indexOf &&
this._customUniform[ind].indexOf(‘sampler’) == -1)

that can fix the problem
anyway i think when we don’t used integer index in array the for in is the best way

https://www.babylonjs-playground.com/#JK6DA4#1

for build the total problem i ping @Deltakosh again for look the code part

BABYLON.CustomMaterial .prototype.ReviewUniform = function(name , arr )  {
    if (name == "uniform") {
        for (var ind in this._newUniforms) {
            if (this._customUniform[ind].indexOf && 
                this._customUniform[ind].indexOf('sampler') == -1) {
                arr.push(this._newUniforms[ind]);
            }
        }
    }
    if (name == "sampler") {
        for (var ind in this._newUniforms) {
            if (this._customUniform[ind].indexOf && 
                 this._customUniform[ind].indexOf('sampler') != -1) {
                arr.push(this._newUniforms[ind]);
            }
        }
    }
    return arr;
}

it is special custom array so if that is useful i can make a PR

Why not just doing something like:

        for (var i = 0; i < this._newUniforms.length; ++i) {
            if (this._customUniform[i].indexOf('sampler') === -1) {
                arr.push(this._newUniforms[i]);
            }
        }

There are the same “for…in” loops over arrays in AttachAfterBind too.

3 Likes

Not have important reason agreed :slight_smile:

the base of language is Typescript and totally it is not standard to changed basic of language in typescript ( Array.Prptotype… ) also the for-in is optimizer than for index ( java reference java - Is there a performance difference between a for loop and a for-each loop? - Stack Overflow) and that support string index too

Don’t forget that this is for java, not JavaScript :blush:

I think a standard for loop will work just fine

2 Likes

Totally agree with for loop and they would be better as not iterating through all the other properties risking issues.

By the way it reminds me of my favorite SO answer:

4 Likes

I love the 2nd answer :slight_smile:

1 Like

should i make PR?

1 Like

please :slight_smile:

1 Like

PR Created

3 Likes

and merged!

5 Likes

hi i think custom material is broke for last marge

Thanks for getting this in! cannot wait to start using 4.2!