facebookarchive / react-360

Create amazing 360 and VR content using React
https://facebook.github.io/react-360
Other
8.73k stars 1.23k forks source link

4+2 seems to be an int: 42 instead of 6. #369

Closed jgwinner closed 6 years ago

jgwinner commented 6 years ago

Description

4+2 seems to be 42 instead of 6.

I thought at first it's my confusion between JavaScript and React, but I'm not so sure now, and want to throw it out there.

Expected behavior

If I have a prop that is a numeric, such as this:

    constructor() {
        super();
        this.props = {
            SizeX: 4,
            SizeY: 4,

I have to invoke it with the props filled in, but they are usually numeric where they need to be:

`

`

I have a loop where I'm calculating a walk path for a maze. The loop code used to look like this:

for (var j = 0; j < this.props.SizeX + 2; j++) {
    unvisited[j] = [];
    for (var k = 0; k < this.props.SizeY + 1; k++)
        unvisited[j].push(j > 0 && j < this.props.SizeX + 1 && k > 0 && (j != here[0] + 1 || k != here[1] + 1));
}

Actual behavior

For the 4x4 maze, which should have resulted in an 'Unvisited' array of 0 ... 6, I got 42!

It was adding this.props.SizeX to 2, and getting 42, then converting that to a number, and running J from 0 to 42 instead of j from 0 to 6. (there's a wall on the left and right of the maze - the size refers to the open cells).

I found even this code doesn't work:

    render() {

        console.log("render, generating maze");
        this.props.SizeX = Math.floor(this.props.SizeX);
        this.props.SizeY = Math.floor(this.props.SizeY);

The line: for (var j = 0; j < this.props.SizeX + 2; j++) { still works j from 0...42.

Is that normal? How do you fix that? I found I have to take the props and assign them to other vars.

        var SizeXPlus2 = 2;
        SizeXPlus2 += Math.floor(this.props.SizeX);
        var SizeYPlus1 = Math.floor(this.props.SizeY ) + 1;

    console.log("Calculating unvisited from 0.." + SizeXPlus2 + " this.props.SizeX:" + this.props.SizeX + " and +2 is " + this.props.SizeX + 2);
    console.log("Calculating unvisited from 0.." + SizeYPlus1 + " this.props.SizeY:" + this.props.SizeY +" and +2 is " + this.props.SizeY+ 2);

produces the following:

Calculating unvisited from 0..6 this.props.SizeX:4 and +2 is 42 Calculating unvisited from 0..5 this.props.SizeY:4 and +2 is 42

This is not just a console.log issue, the actual loops went up to 42. The top edge wall looped around and made the maze impossible to solve :)

Reproduction

Reproduction shown above - I can upload the whole project but it's getting big with all the art.

Solution

No idea what a solution is - I'll cheerfully admit I'm a C++ programmer and may not quite 'get' React-native, but I'm getting a lot better now, so if I'm making a fundamental mistake please help :)

I really hope it's a fundamental mistake, this is a mess.

Note that the plain JavaScript file looks like this:


function maze(x, y) {

...
        console.log("Calculating unvisited from 0.." + (x + 2));
        for (var j= 0; j<x+2; j++) {
            unvisited[j]= [];
            for (var k = 0; k < y + 1; k++) {
                unvisited[j].push(j > 0 && j < x + 1 && k > 0 && (j != here[0] + 1 || k != here[1] + 1));
                console.log("Calculating unvisted at [" + j + "]=" + unvisited[j]);
            }
        }

I thought I missed a this.props or something.

Additional Information

  • 'react-vr' package version:
  • 'react-vr-web' package version: [FILL THIS OUT: get this by running npm list react-vr-web]
F:\ReactVR\WalkInAMaze>npm list react-vr
WalkInAMaze@0.0.1 F:\ReactVR\WalkInAMaze
+-- UNMET PEER DEPENDENCY react@16.0.0
`-- react-vr@2.0.0
npm ERR! peer dep missing: react@16.0.0-alpha.12, required by react-native@0.48.4
F:\ReactVR\WalkInAMaze>npm list react-vr-web
WalkInAMaze@0.0.1 F:\ReactVR\WalkInAMaze
+-- UNMET PEER DEPENDENCY react@16.0.0
`-- react-vr-web@2.0.0
npm ERR! peer dep missing: react@16.0.0-alpha.12, required by react-native@0.48.4
  • Operating System: [FILL THIS OUT: MacOS, Linux, or Windows?] Windows

  • Graphics Card: [FILL THIS OUT: NVIDA, ATI, Intel? Which Driver?] N/A (GTX 1070)

  • Browser: FireFox Nightly - up to date

  • VR Device: In or out.

Amusing visualization of the issue

The 'blocked neighbors' looped around. 1 is player start, 2 is the exit.

JavaScript generated maze:

x1xxxxxxx
x     x x
xxx x x x
x   x   x
x xxxxxxx
x       x
x xxxxxxx
x       2
xxxxxxxxx

React generated maze:

x1xxxxxxx 
x x x x x 
xxxxxxxxx 
x x x x x 
xxxxxxxxx 
x x 
x xxxxxxx 
x x x x 2
x xxx xxx
andrewimm commented 6 years ago

If this is how you are instantiating your object:

<Maze SizeX='4' SizeY='4' CellSpacing='2.5' StartX='0' StartZ='0' />

Then it's just a misunderstanding of JSX, not a React bug. If you quote arguments, they become strings. If you want to pass a different primitive or object value, you need to write code like:

<Maze SizeX={4} SizeY={4} CellSpacing={2.5} StartX={0} StartZ={0} />
jgwinner commented 6 years ago

Wow, that was fast.

Thanks - right, I knew that for array types and didn't think it through.

I guess I thought the constructor would have initialized it to a number, then the string instantiation would have converted to the number established in the constructor.

Nope! And I get why.

I've seen other people's code that used numbers quoted to pass in as properties, but I should know better :) (never trust OPC).

OTOH doing an explicit conversion might be more robust if someone uses it incorrectly. I'll think about that.

== John ==

jgwinner commented 6 years ago

Want me to delete the issue?