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

Bugs
1

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
2

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

1 Like
3

@Wigen: fancy doing a PR?

1 Like
4

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

5

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
6

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

7

Donā€™t forget that this is for java, not JavaScript :blush:

I think a standard for loop will work just fine

2 Likes
8

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
9

I love the 2nd answer :slight_smile:

1 Like
10

should i make PR?

1 Like
11

please :slight_smile:

1 Like
12

PR Created

3 Likes
13

and merged!

5 Likes
14

hi i think custom material is broke for last marge

15

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