› ›

P5.JS Crash When Including a New JS File (packman.js) in the Same HTML File

PersisPersis
edited November 2015 in p5.js Library Questions

I am developing a simple visualization using P5.JS library. I am trying to put a Packman game on top of my visualization by having a div overlap another div element in HTML:

    <head>
      <style type="text/css">
            @font-face {
            font-family: 'BDCartoonShoutRegular';
            src: url('BD_Cartoon_Shout-webfont.ttf') format('truetype');
            font-weight: normal;
            font-style: normal;
          }
        div.pacman {
          position: absolute;
          top: 5px;
          right: 5px;
          width: 463px;
          height: 463px;
          background-color: orange;
          visibility: visible;
          border: 4px solid black;
          z-index: 1;
        }

        div.p5 {
          position: relative;
          top: 10px;
          left: 10px;
          width: 1440px;
          height: 960px;

        }
      </style>

        <script src="libraries/p5.js" type="text/javascript"></script>
         .......
        <script src="Sketch.js" type="text/javascript"></script>

    </head>

    <body>

      <div id="p5_loading" class="loadingclass">
        <div id="p5_loading" class="text">LOADING ...</div>
      </div>

      <div id="p5" class="p5" >
         <div id="pacman" class="pacman">
       </div>

       </div>

    </body>

everything was working fine until I downloaded this Packman game from here and add it to my project as follow:

.........
<body>

  <div id="p5_loading" class="loadingclass">
    <div id="p5_loading" class="text">LOADING ...</div>
  </div>

  <div id="p5" class="p5" >
     <div id="pacman" class="pacman">
   </div>

   </div>

  <script src="pacman.js"></script>
  <script src="modernizr-1.5.min.js"></script>

  <script>

    var el = document.getElementById("pacman");

    if (Modernizr.canvas && Modernizr.localstorage && 
        Modernizr.audio && (Modernizr.audio.ogg || Modernizr.audio.mp3)) {
      window.setTimeout(function () { PACMAN.init(el, "./"); }, 0);
    } else { 
      el.innerHTML = "Sorry, needs a decent browser<br /><small>" + 
        "(firefox 3.6+, Chrome 4+, Opera 10+ and Safari 4+)</small>";
    }
  </script>

</body>

When I run my project, I get the following error: Packman Error

Firebug tells me that "window[o] is undefined".

I have no Idea how to debug this. I am guessing something in pacman.js interferes with something in p5.js.

Any help with debugging this issue is greatly appreciate it!

Tagged:

Best Answers

Answers

  • GoToLoopGoToLoop
    edited November 2015

    You haven't posted your merged ".html" file! So we don't have any clear idea how those scripts are interacting w/ each other! :-@

    Firebug tells me that "window[o] is undefined".

    It means that p5.js library isn't in "global" mode but "instance" mode.
    That happens when p5.js can't find neither setup() nor draw() in the global scope.

    For better understand of that, take a look at this wiki article about it: :-B
    https://GitHub.com/processing/p5.js/wiki/Instantiation-Cases

    P.S.: You've chosen the wrong category. Fixed that already.
    p5.js isn't the same as "JavaScript Mode" (pjs) btW: http://ProcessingJS.org/reference/

  • blindfishblindfish

    @Persis I've just tested a p5js sketch running side by side with that implementation of pacman and in principle it looks like it should work fine. You need to give us more info (such as sketch code) to figure out why this isn't working for you...

  • PersisPersis
    edited November 2015

    Thanks @GoToLoop and @blindfish! here is the code in my sketch.js:

        // Sketch.js
    var mySketch = function(s) {

      var myCanvas;
      var canvasWidth = 1440;
      var canvasHight = 960;
      var backgroundColor = s.color(102, 204, 255);

      var myGrid;
      var gridMargin = 50;
      var xStart = 30;
      var yStart = 550;
      //var gridWidth = canvasWidth - 2 * xStart;
      //var gridHight = canvasHight - 2 * gridMargin;
      var numRows = 3;
      var numCols = 2;
      var blockWidth = 150;
      var blockHight = 150;

      // gridBlock process images
      var emptyProcess_img;
      var NG_img;
      var sGen_img;
      var sEgen_img;
      var sagd_img;
      var sagdCogen_img;

      var myBlockButtons;
      // gridBlock buttons
      var up_img;
      var upGlow_img;
      var upPressed_img;

      var down_img;
      var downGlow_img;
      var downPressed_img;


      var top_right_visImg;
      var bottom_right_visImg;

      // Indicating which tab is active
      var coal_IsActive = false;
      var ng_IsActive = false;
      var sagd_IsActive = true;


      // Tab buttons 
      var sagdButton;
      var ngButton;
      var coalButton;

      var tab_buttonW = 98;
      var tab_buttonH = 46;

      var tab_buttonY = tabs.yStart + 2;

      var sagdTab_buttonX = 650 + tabs.xStart;
      var ngTab_buttonX = 750 + tabs.xStart;
      var coalTab_buttonX = 850 + tabs.xStart;


      /////////////////////////////////////////////////////////////////////////////////////////////////////
      s.preload = function() {

        loadProcessImages();
        loadBlockButtonsImages();
        loadTabsImages();

      };

      s.setup = function() {

        myCanvas = s.createCanvas(canvasWidth, canvasHight);

        myGrid = new Grid(s, xStart, yStart, numRows, numCols, blockWidth,
          blockHight, 5, emptyProcess_img, myBlockButtons);

        setupTabs();
      };

      s.draw = function() {

        s.background(backgroundColor);

        drawTabs();
    //tabs.sagdTab.tab.draw(true);


      };

      s.mousePressed = function() {

          if (sagdTabProperties.tabButton.mouseIsOnButton()) {
            coal_IsActive = false;
            ng_IsActive = false;
            sagd_IsActive = true;
          } else if (naturalGasTabProperties.tabButton.mouseIsOnButton()) {
            coal_IsActive = false;
            ng_IsActive = true;
            sagd_IsActive = false;
          } else if (coalTabProperties.tabButton.mouseIsOnButton()) {
            coal_IsActive = true;
            ng_IsActive = false;
            sagd_IsActive = false;
          }

        }
        //////////////////////////////////////////////////////////////////////////////////////////////////////
      function setupTabs() {

        sagdTabProperties.tabButton = new tabButton(s, sagdTab_buttonX, tab_buttonY, tab_buttonW, tab_buttonH, s.color('white'), "SAGD");

        //sagdButton = new tabButton(s, sagdTab_buttonX, tab_buttonY, tab_buttonW, tab_buttonH, s.color('white'), "SAGD");

        naturalGasTabProperties.tabButton = new tabButton(s, ngTab_buttonX, tab_buttonY, tab_buttonW, tab_buttonH, s.color('white'), "NG");
        coalTabProperties.tabButton = new tabButton(s, coalTab_buttonX, tab_buttonY, tab_buttonW, tab_buttonH, s.color('white'), "COAL");

        tabs.coalTab = new Tab(s, coalTabProperties);
        tabs.naturalGasTab = new Tab(s, naturalGasTabProperties);
        tabs.sagdTab= new Tab(s, sagdTabProperties);
      }

      function drawTabs() {

        // draw sagd, ng and coal tabs
        if (sagd_IsActive) {
          tabs.naturalGasTab.draw( ng_IsActive);
          tabs.coalTab.draw(coal_IsActive);
          tabs.sagdTab.draw(sagd_IsActive);

          myGrid.drawGrid('red', backgroundColor);

        } else if (ng_IsActive) {
          tabs.sagdTab.draw(sagd_IsActive);
          tabs.coalTab.draw(coal_IsActive);
          tabs.naturalGasTab.draw(ng_IsActive);

        } else {
          tabs.sagdTab.draw(sagd_IsActive);
          tabs.naturalGasTab.draw(ng_IsActive);
          tabs.coalTab.draw(coal_IsActive);

        }
        // draw earth and bar chart tabs
        s.image(top_right_visImg, 960 + tabs.xStart, tabs.yStart, 470, 470);
        s.image(button_right_visImg, 960 + tabs.xStart, 480 + tabs.yStart, 470, 470);

      }

      function loadBlockButtonsImages() {
        myBlockButtons = new blockButtons(s);
      }

      function loadProcessImages() {
        emptyProcess_img = s.loadImage("Images/AddProcess.jpg");
        NG_img = s.loadImage("Images/1- NG input v2.jpg");
        sGen_img = s.loadImage("Images/2- steam gen v2.jpg");
        sEgen_img = s.loadImage("Images/2- steam e gen v2.jpg");
        sagd_img = s.loadImage("Images/3- SAGD v2.jpg");
        sagdCogen_img = s.loadImage("Images/3- SAGD COGEN v2.jpg");
      }

      function loadTabsImages() {

        coalTabProperties.activeLayout_img = s.loadImage("Images/CoalTab_Act_v1.png");
        coalTabProperties.deactivelayout_img = s.loadImage("Images/CoalTab_Deact_v1.png");
        coalTabProperties.tabBackground_img = s.loadImage("Images/DefaultTabBackground2.png");

        naturalGasTabProperties.activeLayout_img = s.loadImage("Images/NGTab_Act_v1.png");
        naturalGasTabProperties.deactivelayout_img = s.loadImage("Images/NGTab_Deact_v1.png");
        naturalGasTabProperties.tabBackground_img = coalTabProperties.tabBackground_img;

        sagdTabProperties.activeLayout_img = s.loadImage("Images/SAGDTab_Act_v1.png");
        sagdTabProperties.deactivelayout_img = s.loadImage("Images/SAGDTab_Deact_v1.png");
        sagdTabProperties.tabBackground_img = coalTabProperties.tabBackground_img;

        top_right_visImg = s.loadImage("Images/EarthVis_Tab.png");
        button_right_visImg = top_right_visImg;
      }

      /* function CarTypes(name) {
         if (name == 1) {
           return "Honda";
         } else {
           return "Sorry, we don't sell " + name + ".";
         }
       }*/
    };

    var mySketch_one = new p5(mySketch, 'p5');

    And here is how the index.html looks like:

        <!DOCTYPE html>
    <html>

    <head>
      <meta charset="UTF-8">
      <title>Untitled</title>


      <style type="text/css">
            @font-face {
            font-family: 'BDCartoonShoutRegular';
            src: url('BD_Cartoon_Shout-webfont.ttf') format('truetype');
            font-weight: normal;
            font-style: normal;
          }
        div.pacman {
          position: absolute;
          top: 5px;
          right: 5px;

          width: 463px;
          height: 463px;
          background-color: orange;
          visibility: visible;
          border: 4px solid black;
          z-index: 1;
        }

        div.p5 {
          position: relative;
          top: 10px;
          left: 10px;
          width: 1440px;
          height: 960px;

        }
      </style>

      <script type="text/javascript">
        function showFrontLayer() {
          //document.getElementById('bg_mask').style.visibility = 'visible';
          document.getElementById('frontlayer').style.visibility = 'visible';
        }

        function hideFrontLayer() {
          //document.getElementById('bg_mask').style.visibility = 'hidden';
          document.getElementById('frontlayer').style.visibility = 'hidden';
        }
      </script>

      <script src="libraries/p5.js" type="text/javascript"></script>

      <!-- Uncomment the lines below to include extra p5 libraries -->
      <script src="libraries/p5.dom.js" type="text/javascript"></script>
      <script src="libraries/p5.sound.js" type="text/javascript"></script>
      <script src="Global.js" type="text/javascript"></script>
      <script src="Tab.js" type="text/javascript"></script>
      <script src="TransparentButton.js" type="text/javascript"></script>
      <script src="Card.js" type="text/javascript"></script>
      <script src="GridBlock.js" type="text/javascript"></script>
      <script src="GridButtons.js" type="text/javascript"></script>
      <script src="Grid.js" type="text/javascript"></script>
      <script src="Sketch.js" type="text/javascript"></script>
    </head>

    <body>
      <input type="text" value="testing text" />
      <input type="button" value="Show front layer" onclick="showFrontLayer();" /> Click 'Show front layer' button
      <br/>
      <br/>
      <br/>

      <div id="p5_loading" class="loadingclass">
        <div id="p5_loading" class="text">LOADING ...</div>
      </div>
      <div id="p5" class="p5" >
         <div id="pacman" class="pacman">
        <!--  <input type="button" value="Hide front layer" onclick="hideFrontLayer();" /> -->
        </div>

      </div>

      <script src="pacman.js"></script>
      <script src="modernizr-1.5.min.js"></script>

      <script>

        var el = document.getElementById("pacman");

        if (Modernizr.canvas && Modernizr.localstorage && 
            Modernizr.audio && (Modernizr.audio.ogg || Modernizr.audio.mp3)) {
          window.setTimeout(function () { PACMAN.init(el, "./"); }, 5000);
        } else { 
          el.innerHTML = "Sorry, needs a decent browser<br /><small>" + 
            "(firefox 3.6+, Chrome 4+, Opera 10+ and Safari 4+)</small>";
        }

      </script>


    </body>

</html>

    Please let me know if you need to see the content of any other js files.

  • GoToLoopGoToLoop
    edited November 2015

    You should place <script src=Sketch.js></script>
    much before than <script src=libraries/p5.js defer></script>.

    As mentioned, if "p5.js" runs and doesn't find any setup() nor draw() it enters "instance mode". :|

  • PersisPersis

    I just did that and moved <script src=Sketch.js></script> before <style type="text/css"> but now I get: "Uncaught ReferenceError: p5 is not defined " in Sketch.js at line 191!

  • GoToLoopGoToLoop
    edited November 2015

    Sorry, I haven't paid attention that "Sketch.js" was using "instance mode". b-(
    Then it's exactly the opposite: it should happen well after <script src=libraries/p5.js></script>! 3:-O

    Glancing more intently over your error screenshot, it seems image loading inside preload() is failing.

    I wonder whether we can call other functions from there.
    And where do coalTabProperties, naturalGasTabProperties, sagdTabProperties, etc. come from?
    I can't find their instantiation inside "Sketch.js"! :-/

  • PersisPersis
    edited November 2015

    No worries! have a Glonal.js file which includes those variables:

      var tabs = {

    xStart: 5,
    yStart: 5,

    tabWidth: 950,
    tabHight: 950,

    coalTab: null,
    naturalGasTab: null,
    sagdTab: null
  };
  var coalTabProperties = {
    tabButton: null,

    activeLayout_img: null,
    deactivelayout_img: null,
    tabBackground_img: null,
    xStart: tabs.xStart,
    yStart: tabs.yStart,

    tabWidth: tabs.tabWidth,
    tabHight: tabs.tabHight
  };

  var naturalGasTabProperties = {
    tabButton: null,

    activeLayout_img: null,
    deactivelayout_img: null,
    tabBackground_img: null,
    xStart: tabs.xStart,
    yStart: tabs.yStart,

    tabWidth: tabs.tabWidth,
    tabHight: tabs.tabHight
  };

  var sagdTabProperties = {
    tabButton: null,
    activeLayout_img: null,
    deactivelayout_img: null,
    tabBackground_img: null,
    xStart: tabs.xStart,
    yStart: tabs.yStart,

    tabWidth: tabs.tabWidth,
    tabHight: tabs.tabHight
  };
  • GoToLoopGoToLoop
    edited November 2015

    You still need to take in consideration whether "Global.js" was fully loaded before preload() happens.

    But my theory now is preload() is somehow confused about using global variables to receive those loadImage() functions whiole being in "Instance Mode".

    Sorry I can only speculate b/c I still need to study the actual mechanism behind preload().
    It's more like a "hack" in order to allow "synchronous" loading of resources so we can skip callbacks. :ar!

  • GoToLoopGoToLoop
    edited November 2015

    I did some tests w/ preload() below:
    http://CodePen.io/anon/pen/GpPEGX?editors=101

    It doesn't show any problems calling another function from within preload().
    Neither dealing w/ global variables created from another script. :-@
    No matter whether sketch is in global or instance mode. I've tested global 1st. :(|)

  • PersisPersis

    I just realized that as soon as I add <script src="pacman.js"></script> in incex.html then I have a problem and that makes my p5 code get stuck at "LOADING ... !" phase meaning it makes my s.preaload crash!

    Can it be possible that pacman.js is somehow using a variable with the same name as some variable in p5.js and somehow one overwrites another which causes the crash?

  • GoToLoopGoToLoop
    edited November 2015

    Can it be possible that pacman.js is somehow using a variable with the same name as some variable in p5.js and somehow one overwrites another which causes the crash?

    Both your "Sketch.js" & "pacman.js" are encapsulated on 1 variable each: mySketch & Pacman.
    And I don't think those big variable names inside "Global.js" would be overriding anything. :-?

    There are some more global variables like PACMAN, KEY, UP. DYING, etc. though.
    But again, they don't seem to crash w/ anything I guess...

  • PersisPersis

    I just commented out s.preload in Sketch.js and moved its content to s.setup and now my program works but with some errors related to AudioDestinationNode.

    Packman Error 2

    I think p5.js and pacman.js create elemets on html that have the same name and might conflict with each other. For example if you look at the following code from pacman.js:

        function load(name, path, cb) { 

        var f = files[name] = document.createElement("audio"); // Here

        progressEvents[name] = function(event) { progress(event, name, cb); };

        f.addEventListener("canplaythrough", progressEvents[name], true);
        f.setAttribute("preload", "true"); // Here
        f.setAttribute("autobuffer", "true");
        f.setAttribute("src", path);
        f.pause();        
    };

    Look at the lines with comment "//Here". maybe the "audio" element and "preload" attribute are conflicting with the audio and preload from p5 ?

  • GoToLoopGoToLoop
    edited November 2015

    Why do you have "p5.sound.js" & "p5.dom.js" loaded? :-@
    Your "Sketch.js" doesn't seem to use anything from them! #-o
    Isn't "modernizr-1.5.min.js" already dealing w/ sound and other things for "pacman.js"? :-w

  • GoToLoopGoToLoop
    edited November 2015

    Maybe the "audio" element and "preload" attribute are conflicting with the audio and preload() from p5 ?

    In f.setAttribute("preload", "true");, variable f refers to an HTMLAudioElement:
    https://developer.Mozilla.org/en-US/docs/Web/API/HTMLAudioElement

    It's got nothing to do w/ p5's own preload() callback. #:-S

  • blindfishblindfish

    Can it be possible that pacman.js is somehow using a variable with the same name as some variable in p5.js and somehow one overwrites another which causes the crash?

    In standard mode yes - it definitely does have name clashes (e.g. LEFT). It's easy enough to check by doing console.log(window). You then see all the properties and methods attached to window (the global namespace in the browser context). Chrome helpfully highlights non-native globals...

    I've often moaned on here about the dangers of polluting the global namespace; something that p5js does with wild abandon (in standard mode) :((

    The problems you're experiencing are exactly why standard mode should be avoided. The moment you start mixing with source-code that shows equal disdain for good practice you will run into major issues. Even if you don't hit the problem in development you may experience it after minifying separate files only to discover variables in different files have been given the same short name in the global namespace...

    It's probably worth checking whether any of the other p5js libraries are polluting with globals; though in instance mode I believe you're safe on the p5js front, and looking at pacman.js I think I found your problem...

    I notice that pacman.js adds a clone method directly to the Object.prototype (line #1256). That's a particularly nasty thing to do. It basically adds a clone method to every single Object (and 'everything in JS is an Object' (well almost)). It means every object has an extra property that, for example, will be output in a for in loop unless necessary precautions have been taken. If the code in the for in loop isn't expecting this (which it won't)... 8-X 8-X 8-X

    There's no way to protect against this unless you fix for in loops that don't protect themselves from this type of abuse; so it would be far easier to fix the pacman.js code directly. This type of bad practice is pure, unadulterated evil >:) X(

  • PersisPersis
    edited November 2015

    @GotoLoop: Why do you have "p5.sound.js" & "p5.dom.js" loaded?

    I am planning to use them in the future ( probably going to use p5.play and p5.particle too) so I need to fix the error related to the audio library.

    @blindfish: I notice that pacman.js adds a clone ...

    So basically you are saying that I should make a clone method for Pacman.MAP and remove the Object.prototype.clone?

    Thanks guys for all your help!

  • GoToLoopGoToLoop
    edited November 2015

    I am planning to use them in the future...

    Even so, you should still check whether the loading of "p5.sound.js" is the culprit! L-)
    If it happens to be so, you should modify "pacman.js" to use "p5.sound.js" instead of its own approach.

    ... and remove the Object.prototype.clone()?

    Although it's frowned upon to add functionality directly into JS' built-in objects, that particular additional feature seems innocuous iMO. (:|

  • blindfishblindfish
    edited November 2015

    Although it's frowned upon to add functionality directly into JS' built-in objects, that particular additional feature seems innocuous iMO.

    @GoToLoop: You don't know the risks of adding to Object.prototype (or in fact any Object)??!!!?? I'm speechless :-O

    Try this [edit - simplified my example code. You don't need p5js to demonstrate the issue. Just run this in your browser console]:

    Object.prototype.evilMethod = function() { 
    console.info("I am evil");
};

(function() {
     var myArray = [1,2,3,4];

     for(var item in myArray) {
        console.info(myArray[item]);    

     }
})();

    That's hardly innocuous...

  • PersisPersis
    edited November 2015

    Ok so you are saying that I should leave Object.prototype.clone() as it is and focus on p5.sound.js to fix the audio error?

    I am looking into figuring out how I can replace Object.prototype.clone() so I can have map = Pacman.MAP.clone() working at line 590.

    I still wonder why p5.preload() crashes when I include pacman.js in my project. :-/

  • blindfishblindfish

    Looks exactly like what I suspected: the preload code is full of unprotected for in loops that, thanks to EvilPacMan, now have an extra unexpected item to digest :/

  • GoToLoopGoToLoop
    edited November 2015 Answer ✓

    You don't know the risks of adding to Object.prototype...

    I do! Although I've never done it myself, yet... >:)

    But indeed new properties are born w/ their attributes:
    enumerable = writable = configurable = true. :-SS
    So they do show up in for...in loops & Object.keys() as long as their enumerable = true. :-S

    In order to fix that, we can use static method Object.defineProperty() on them:
    https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty

    And we can check whether it worked w/ method propertyIsEnumerable():
    https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/propertyIsEnumerable

    Object.prototype.clone = function () {
  const newObj = this.constructor == Array? [] : {};
  var v, i;

  for (i in this)  if (i != 'clone')
    v = this[i], newObj[i] = v && typeof v == 'object'? v.clone() : v;

  return newObj;
};

console.log(Object.prototype.propertyIsEnumerable('clone')); // still true...
Object.defineProperty(Object.prototype, 'clone', { enumerable: false });
console.log(Object.prototype.propertyIsEnumerable('clone')); // now it's false!
  • GoToLoopGoToLoop
    edited November 2015

    ... the preload()'s code is full of unprotected for...in loops that...

    Oh, I didn't notice that! #-o I don't use for...in nor Object.keys() that much... :-\"
    Object.defineProperty() can promptly hide that "invasive" clone() though.
    But the ideal should be having a separate clone() function unattached from any JS's built-in objects. >-)

  • GoToLoopGoToLoop
    edited November 2015

    Try this... You don't need p5js to demonstrate the issue. Just run this in your browser console.

    Actually if we hit F12 to open JS' console and run just this snippet while right at https://forum.Processing.org/two/ page:

    var myArr = [Math.PI, Math.E, Math.SQRT2, Math.SQRT1_2];
for (var property in myArr)  console.info(myArr[property]);

    You'll realize in awe JS' built-in Array object has already been "hacked" by forum's "global.js" file! >:)
    More specifically methods: sum(), max() & min() were added into Array.prototype! @-)

    But fret not, Object.defineProperties() static method for the rescue: :D
    https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperties

    var noEnum = { enumerable: false };
Object.defineProperties(Array.prototype, { sum: noEnum, max: noEnum, min: noEnum });

    And while we're at it, let's hide your Object.prototype.evilMethod() too: O:-)

    Object.defineProperty(Object.prototype, 'evilMethod', noEnum);

    Now just re-run myArr's loop and you'll see those additional props. have simply vanished. :-bd
    Well, technically they're still there, but well hidden from critical places. :-$
    And at least they won't show up their ugly faces in neither for...in nor Object.keys()! :P

  • blindfishblindfish
    edited November 2015

    @GoToLoop: yep that's all modern best practice when adding to JS native objects; but you don't always have control over the offending code (the examples you mention on the forum - which I'd also spotted - being a case in point)... If you know your library is going to be run alongside code of unknown quality (as will definitely be the case with anything in p5js since it's used by beginners) it's wise to protect against this issue in the first place.

    for in loops are known to be troublesome so I tend to just use standard for loops. If you prefer the for in syntax you need to guard against inherited elements. ES6 also introduces a new for of loop that doesn't share this problem; but isn't supported by all browsers...

    "use strict"; 

Object.prototype.evilMethod = function() { 
    console.info("I am evil");
};

(function() {
     var myArray = [1,2,3,4];

    console.info("traditional");
     for(var i= 0, len = myArray.length; i < len ; i++) {
            console.info(myArray[i]);    
     }

    console.info("protected");
     for(var item in myArray) {
         if(myArray.hasOwnProperty(item)) {
            console.info(myArray[item]);    
         }

     }

    console.info("ES6: may not work in all browsers");
     for(let item of myArray) {
            console.info(item);
     }

     console.info("unprotected: don't do this!");
     for(var item in myArray) {
            console.info(myArray[item]);    
     }

})();

    [edit: expanded examples and explanation]

  • blindfishblindfish

    I'm in the mood for raising issues...

  • GoToLoopGoToLoop
    edited November 2015

    If you prefer the for...in syntax...

    I sure do prefer that 1! But since regular for (;;) is faster, I avoid for...in like the plague! :P

    ES6 also introduces a new for...of loop that doesn't share this problem; ...

    It's even more tempting b/c it's pretty much Java's own "enhanced" for ( : ) loop.
    But I never use it in JS for arrays b/c it's based on Symbol.iterator (@@iterator); thus slower : :(
    https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols
    Java is much smarter and doesn't apply Iterator when dealing w/ arrays! \m/

    ... but isn't supported by all browsers...

    If you check its compatibility table, right now only IE doesn't support for...of loops! :O)

  • GoToLoopGoToLoop
    edited November 2015

    I'm in the mood for raising issues...

    The cruel reality is that even p5.js dirties built-in native objects.
    More precisely Uint8ClampedArray w/ polyfill method slice() from Array.prototype.slice().
    And as naïve & slack as "evil pacman.js" did w/ its own clone()'s insertion into Object.prototype.

    We can check that out at the end of "shim.js": https://GitHub.com/processing/p5.js/blob/master/src/core/shim.js

    They don't even check whether slice() was already implemented!
    Which is currently true for Firefox 38 for now. Awaiting for others to follow suit too... (:|

    For bug test-drive, go to http://p5js.org/, hit F12, go to console tab and type in: :@)
    Uint8ClampedArray.prototype.propertyIsEnumerable('slice'); // true for p5js.org!

    The lesson here is clear: if we insist to "patch" native objects w/ some polyfills and other "innovative" methods, at least make sure they're non-enumerable; as all native methods follow suit already! :-w

    Here's some more appropriate approach I've come up w/ for such "invasive" band-aids: :-\"

      (function () {
    if (Uint8ClampedArray && !Uint8ClampedArray.prototype.slice)
      Object.defineProperty(Uint8ClampedArray.prototype, 'slice', {
        value: Array.prototype.slice,
        writable: true, configurable: true, enumerable: false
      });
  }());
  • GoToLoopGoToLoop
    edited November 2015 Answer ✓

    I've also pulled some fix request, but for "pacman.js": :bz
    https://GitHub.com/daleharvey/pacman/pull/9

  • PersisPersis
    edited November 2015

    This was a great discussion !( though I am a beginner in JavaScript and don't understand half of the material mentioned in this thread @-) )

    Thanks for all your help! =D>

  • discomandiscoman
    edited February 2016

    @GoToLoop I looked for other bad function injections into native built-in objects in p5.js for Object.prototype, Array.prototype, and Uint8ClampedArray.prototype. Luckily the one specified by you is the only one showing this behaviour of altering the native objects. Should I fix this posting issue and resolution provided by you, just wanted to know if you feel like suggesting something.

  • GoToLoopGoToLoop
    edited February 2016

    By all means, go for it! Just make sure to check 1st whether a native slice() already exists before hacking it: if (!Uint8ClampedArray.prototype.slice) => https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/slice

    And btW, another huge flaw w/ p5.js is that it's incompatible w/ typed arrays in general.
    p5.js library relies on obj instanceof Array in order to check for Array type throughout its source code.

    However that's very limiting b/c it leaves out 100% of typed arrays, arguments, and any other containers which happen to have numerical indices, but don't inherit from Array. :-O

    Some utility function like the 1 below for replacing obj instanceof Array is a lil' step onwards at making p5.js embracing more of what JS can offer: :-B

    function isArrayLike(obj) {
  return obj.length >= 0 && typeof target === 'object';
}
  • discomandiscoman

    TypedArray.prototype.slice() is not supported by many browsers but according to its page on MDN

    This method has the same algorithm as Array.prototype.slice().

    And therefore even if we check for TypedArray.prototype.slice() availability it would hardly matter, but still would be good to include if (!Uint8ClampedArray.prototype.slice) ,it will make it easy to understand for others.

    And this shim was taken from here, in "Extending Canvas related object prototypes" small paragraph towards the end, if you feel like taking a look.

    Just curious as you wrote -

    And btW, another huge flaw w/ p5.js is that it's incompatible w/ typed arrays in general. p5.js library relies on obj instanceof Array in order to check for Array type throughout its source code.

    I guess this maybe because typed array and specifically its methods are not fully supported by all browsers and our code would have been full of polyfills for specific browser compatibility like this we have written for firefox and chrome compatibility. Please correct me if I am wrong.

  • GoToLoopGoToLoop
    edited February 2016

    And this shim was taken from here, in "Extending Canvas related object prototypes" small paragraph towards the end...

    The 1 posted in this forum I've made it myself. B-) The 1 posted from that link is buggy: 8-|

    if(typeof Uint8ClampedArray !== 'undefined'){
    Uint8ClampedArray.prototype.slice = Array.prototype.slice; //Firefox and Chrome
} else if(typeof CanvasPixelArray!== 'undefined') {
    CanvasPixelArray.prototype.slice = Array.prototype.slice; //IE10 and IE9
} else {
    // Deprecated browser
}

    Besides being very bloaty, checking for ancient deprecated browsers which p5.js probably won't work with, it also re-introduces the "enumerable" bug in this expression:
    Uint8ClampedArray.prototype.slice = Array.prototype.slice;

    Creating properties via the assignment = operator sets all of its descriptors to true, including the aforementioned "enumerable", which shows up at for ... in & Object.keys()! :-&

    In order to create or redefine some property w/ the exact descriptors we want, we use Object.defineProperty() instead: https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperties

    Uint8ClampedArray.prototype.slice || 
  Object.defineProperty(Uint8ClampedArray.prototype, 'slice', {
    value: Array.prototype.slice,
    writable: true, configurable: true, enumerable: false
  });

    And therefore even if we check for TypedArray.prototype.slice() availability it would hardly matter,

    When polyfilling for any official API, we should always check whether it's already present natively.
    And even though the MDN link states that only Firefox 38+ got TypedArray.prototype.slice(), it doesn't seem to be true anymore! :-@

    In my SlimJet 64-bit browser 7.0.5.0, based on Chromium 47.0.2526.73, if I type in Uint8ClampedArray.prototype.slice in its console, it displays: function slice() { [native code] }! $-)
    It means it's also implemented natively in latest Chrome family browsers and they don't need that polyfill anymore! \m/

    I guess this maybe because typed array and specifically its methods are not fully supported by all browsers...

    Typed arrays were introduced at the same pre-historic age as HTML5! And thus they're ECMA 5.
    They exist since Firefox 4, Chrome 7, Opera 11.6, Safari 5.1 and IE10.
    The only 1 little exception is Uint8ClampedArray, which is IE11 instead of IE10.

    1. https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray#Browser_compatibility
    2. https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8ClampedArray#Browser_compatibility

    IMO p5.js shouldn't ignore such ancient feature. It's a matter of checking for arraylike/pseudo arrays rather than exactly for regular arrays only. *-:)

    function isArrayLike(obj) {
  return obj.length >= 0 && typeof target === 'object';
}
Sign In or Register to comment.