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

Statement is always true #6644

Closed
yandeu opened this issue Oct 7, 2023 · 14 comments
Closed

Statement is always true #6644

yandeu opened this issue Oct 7, 2023 · 14 comments

Comments

@yandeu
Copy link

yandeu commented Oct 7, 2023

https://github.com/photonstorm/phaser/blob/cd1854630eb8fec5c43e59a64f005a701c5c16f7/src/renderer/webgl/WebGLRenderer.js#L26

This statement is always true. typeof always return a non-empty string.

I guess it should be something like:

if (typeof WEBGL_DEBUG !== 'undefined' && (WEBGL_DEBUG === true || WEBGL_DEBUG === 'true'))
@samme
Copy link
Contributor

samme commented Oct 10, 2023

This is on purpose, although I don't think I can explain it perfectly.

@photonstorm
Copy link
Collaborator

This isn't actual code, it's never run, so these changes are not required. It's part of the DefinePlugin for Webpack, which is just a pure text replacement plugin. The actual code that ends-up in the built files doesn't include the typeof at all. See here: https://webpack.js.org/plugins/define-plugin/

@yandeu
Copy link
Author

yandeu commented Oct 10, 2023

The actual code that ends-up in the built files

Maybe not if you build it with webpack. But it is shipped to npm. And there, you can't disable loading phaser3spectorjs.

phaser-npm

@yandeu
Copy link
Author

yandeu commented Oct 10, 2023

 "dependencies": {
   "canvas": "^2.10.2",
   "jsdom": "^20.0.3",
+  "phaser3spectorjs": "npm:empty-npm-package@1.0.0"
 },

Solved it by aliasing phaser3spectorjs with empty-npm-package. This way it will just load the empty module.

yandeu added a commit to geckosio/phaser-on-nodejs that referenced this issue Oct 10, 2023
@photonstorm
Copy link
Collaborator

I can't see how using it from npm will make any difference as it uses the dist build when you import from npm, not the source, so the conditional code-swap has already taken place.

If you're directly importing specific source files into your game, that's a different issue entirely. Building from source should be done with webpack and DefinePlugin or an equivalent that does the exact same thing, otherwise it will break all of the conditional features (canvas/webgl, audio, debug, etc) used through-out the codebase.

@yandeu
Copy link
Author

yandeu commented Oct 10, 2023

it uses the dist build

Not on node.js, there it gets the files from ./src/phaser.js (what I prefere), see:
https://github.com/photonstorm/phaser/blob/master/package.json#L11

@photonstorm
Copy link
Collaborator

Ok, fair enough, if you're crazy enough to be running Phaser on the server, then yes, it will pull it from the source. Just understand that none of the conditional checks will work at all (and there are loads of them). They're build flags, should be treated as such and handled as such.

@yandeu
Copy link
Author

yandeu commented Oct 10, 2023

I know. I have worked with the definePlugin before.

Well, I do not personally run Phaser on the server, but I maintain phaser-on-nodejs :)

But I personally recommend using the following approach, which works with and without webpack, instead of that weird approach with the typeof as prefix in https://github.com/photonstorm/phaser/blob/master/config/webpack.config.js#L39.

// webpack.config.cjs

const path = require('path')
const webpack = require('webpack')

module.exports = {
  entry: './index.js',
  output: {
    filename: 'bundle.js',
    path: path.resolve(__dirname)
  },
  plugins: [
    new webpack.DefinePlugin({
      WEBGL_DEBUG: JSON.stringify(true)
    })
  ]
}
// works with and without webpack
if (typeof WEBGL_DEBUG !== 'undefined') {
  console.log('is debug')
} else {
  console.log('normal')
}

// or even better
if (typeof WEBGL_DEBUG !== 'undefined' && WEBGL_DEBUG === true) {
  console.log('is debug')
} else {
  console.log('normal')
}

If you bundle the above with webpack, it will produce:

console.log("is debug"),console.log("is debug");

If you run node index.js, it will output:

normal
normal

@photonstorm
Copy link
Collaborator

You watch - the moment I do this, they'll change DefinePlugin and break it from working. I'm just doing it the way they recommend, however arse-backwards that is.

@yandeu
Copy link
Author

yandeu commented Oct 10, 2023

😅👍🏻

remarkablemark added a commit to remarkablegames/phaser-jsx that referenced this issue Jan 8, 2024
photonstorm added a commit that referenced this issue Feb 21, 2024
…ow those crazy souls who import source directly on node to have a better life #6644
@photonstorm
Copy link
Collaborator

So I tested this today, by changing the webpack config as such:

            new webpack.DefinePlugin({
                CANVAS_RENDERER: JSON.stringify(true),
                WEBGL_RENDERER: JSON.stringify(true),
                WEBGL_DEBUG: JSON.stringify(true),
                EXPERIMENTAL: JSON.stringify(true),
                PLUGIN_3D: JSON.stringify(false),
                PLUGIN_CAMERA3D: JSON.stringify(false),
                PLUGIN_FBINSTANT: JSON.stringify(false),
                FEATURE_SOUND: JSON.stringify(true)
            }),

and the checks to:

if (typeof WEBGL_RENDERER !== 'undefined')
{
    renderWebGL = require('./ContainerWebGLRenderer');
}

and it does not work. The DefinePlugin fails to replace the code correctly and it always gets included.

So while it may fix it for node, it breaks it for the actual build. I tried changing it to: if (typeof WEBGL_RENDERER !== 'undefined' && WEBGL_RENDERER === true) and it still didn't work, DefinePlugin still included it, no matter what the flag was set to in the webpack config.

Even worse, ESLint is now moaning like hell about undef errors everywhere.

So, sadly, I'm reverting this change.

@yandeu
Copy link
Author

yandeu commented Feb 21, 2024

It is easier for me to reply in a video:

phaser-webpack-define.mp4

@photonstorm
Copy link
Collaborator

Try this: In webpack.config.js if you do: "typeof PLUGIN_CAMERA3D": JSON.stringify(true) (the default is 'false'), then do npm run build you'll get a build/phaser.js file that has the Camera3D plugin, as expected. You can even see it on the webpack build results, don't even need to inspect the js file, but you can if you like.

Now modify src/phaser.js line 61 to this:

if (typeof PLUGIN_CAMERA3D !== 'undefined')

Set the flag in the webpack config back to false and run build again. The Camera3D plugin will still be in there.

I tried changing the webpack config to: "PLUGIN_CAMERA3D": JSON.stringify(false), and also PLUGIN_CAMERA3D: JSON.stringify(false), without quotes, and in both cases the addition of !== 'undefined' makes the replacement fail. Remove that, and the replacement works fine.

@photonstorm
Copy link
Collaborator

On the plus side, I found a way to stop ESLint moaning about the undefs, so I'm not worried about that any longer!

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

No branches or pull requests

3 participants