Skip to content

Conversation

@fernandojsg
Copy link
Collaborator

Modified the Detector.addGetWebGLMessage so you could pass {version: 'webgl2'} and it will create the correct message and link to https://get.webgl.org/webgl2

image

@fernandojsg fernandojsg requested a review from mrdoob July 18, 2018 23:14
@mrdoob mrdoob modified the milestones: r96, r95 Jul 19, 2018

//

if ( ! Detector.webgl2 ) Detector.addGetWebGLMessage( { version: 'webgl2' });
Copy link
Collaborator

@Mugen87 Mugen87 Aug 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would add Detector.addGetWebGL2Message() instead in order to avoid the parameter object.

Copy link
Owner

@mrdoob mrdoob Aug 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mugen87 @mrdoob what do you think about adding a addGetWebGLMessageByVersion to avoid duplicating code?

addGetWebGLMessageByVersion: function ( parameters, version ) {

  var parent, id, element;

  parameters = parameters || {};

  parent = parameters.parent !== undefined ? parameters.parent : document.body;
  id = parameters.id !== undefined ? parameters.id : 'oldie';

  element = Detector.getWebGLErrorMessage( version );
  element.id = id;

  parent.appendChild( element );

},

addGetWebGL2Message: function ( parameters ) {
  
  Detector.addGetWebGLMessageByVersion( parameters, 'webgl2' );

},

addGetWebGLMessage: function ( parameters ) {

  Detector.addGetWebGLMessageByVersion( parameters, 'webgl' );

}

Do you think we should do something similar with Detector.getWebGLErrorMessage/getWebGL2ErrorMessage ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think about adding a addGetWebGLMessageByVersion to avoid duplicating code?

I think that's fine. But I would name it _addGetWebGLMessageByVersion() or make a function instead (and move the whole code into a closure).

Do you think we should do something similar with Detector.getWebGLErrorMessage/getWebGL2ErrorMessage ?

If it makes the code easier to read, why not? 😊

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@fernandojsg
Copy link
Collaborator Author

@Mugen87 @mrdoob modified as proposed

@mrdoob mrdoob modified the milestones: r96, r97 Aug 29, 2018
@fernandojsg
Copy link
Collaborator Author

/friendly-ping @mrdoob

@mrdoob
Copy link
Owner

mrdoob commented Sep 26, 2018

Ended up refactoring Detector.js completely. #14967

@mrdoob mrdoob closed this Sep 26, 2018
@fernandojsg fernandojsg deleted the detector branch September 26, 2018 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants