vmars316 Posted June 7, 2016 Share Posted June 7, 2016 Hello & Thanks , I am having trouble with my phone version of BenghaziGame . My viewport : <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1"/> Also , I changed the camvas dimensions from this: <canvas id="canvas" width="800" height="550" style="background-color:#992D2D"></canvas> to this: <canvas id="canvas" width="360" height="400" style="background-color:#992D2D"></canvas> And it looks like I'll need to adjust a bunch of other stuff to make it visually acceptable . Running the program on desktop , it all runs fine . But running same code on phone , the collision detection doesn't work , and the prog runs very , very slow . I could use some recommendations on what can I tweak to improve phone I am running it on my Lumia 640 windows phone . http://liesandcowpies.com/ gives options to run on phone or desktop . Thanks Link to comment Share on other sites More sharing options...
Ingolme Posted June 8, 2016 Share Posted June 8, 2016 There's a lot that needs to be improved in your code, starting with indentation and proper commenting so that your code is easy to read. I notice a large amount of your variables are not declared using var. This means they're in the global scope. It is inefficient and also leaves the risk of its value being overwritten. Your intervals are too short: interval = setInterval(updateGameArea, 5); This is expecting the browser to run the operation 200 times per second. Most mobile devices cap the browser at 60 redraws per second. The time intervals should be no less than 17 milliseconds. If you want precision timing, look into requestAnimationFrame() it's faster and more efficient than setInterval(). This is probably one of the biggest factors in your program being slow on mobile devices. You have quite a lot of messy code. Just replace this: var TargetImages = (function () { function TargetImages() { // Big block of code } // endof function TargetImages() return TargetImages; })(); // endof var TargetImages = (function () With this: function TargetImages() { // Big block of code } It's more readable and more efficient. Finally the block of code that's probably the biggest culprit in slowing your game down is this one: this.update = function() { ctx.drawImage(allImgs[allImgsCnt], this.x, this.y, this.width, this.height); this.x = this.x + (this.speedX * this.directionX); // always = +1 or -1 // idTag = this.idTag; if(this.x > canvas.width - 2) { this.directionX = -1; // change direction this.x = canvas.width - 2; this.tripsCnt++; totalTrips++; } if(this.x < -64 ) { this.directionX = 1; // change direction this.x = 0; this.tripsCnt++; totalTrips++; } if(this.tripsCnt > this.tripsMax) { // time to change sprites if(this.idTag == "tru01") { this.speedX = 0; tru01.tripsCnt = 0; this.visible=false ; lie01.speedX=1 ; lie01.visible=true ; // idTag = "lie01"; } if(this.idTag == "tru02") {this.speedX = 0; tru02.tripsCnt =0; this.visible=false ; lie02.speedX=2 ; lie02.visible=true ; // idTag = "lie02"; } if(this.idTag == "tru03") {this.speedX = 0; tru03.tripsCnt =0; this.visible=false ; lie03.speedX=1 ; lie03.visible=true ; // idTag = "lie03"; } if(this.idTag == "tru04") {this.speedX = 0; tru03.tripsCnt =0; this.visible=false ; lie04.speedX=1 ; lie04.visible=true ; // idTag = "lie03"; } if(this.idTag == "lie01") {this.speedX = 0; lie01.tripsCnt =0; this.visible=false ; tru01.speedX=1 ; tru01.visible=true ; // idTag = "tru01" ; } if(this.idTag == "lie02") {this.speedX = 0; lie02.tripsCnt =0; this.visible=false ; tru02.speedX=1 ; tru02.visible=true ; // idTag = "tru02" ; } if(this.idTag == "lie03") {this.speedX = 0; lie03.tripsCnt =0; tru03.speedX=1 ; tru03.visible=true ; // idTag = "tru03" ; } if(this.idTag == "lie04") {this.speedX = 0; lie04.tripsCnt =0; tru04.speedX=1 ; tru04.visible=true ; // idTag = "tru04" ; } } for (var i = cowpies.length - 1; i >= 0; i--) { thisPie = i; if (cowpies[thisPie].idActive) { if(self.visible) { if (cowpies[thisPie].y < this.y + this.height && cowpies[thisPie].y + cowpies[thisPie].height > this.y) { if (cowpies[thisPie].x < this.x + this.width && cowpies[thisPie].x + cowpies[thisPie].width > this.x ) { thisIdTag = this.idTag ; var findsIt = 0 ; var playsIt = 0 ; var yestru = thisIdTag.startsWith("tru"); var yeslie = thisIdTag.startsWith("lie"); if(yestru) { switch(this.idTag) { case "tru01":tru01.visible = false ; tru01.tripsCnt =0 ; tru01.x = -60; this.speedX = 0; lie01.visible=true ; lie01.tripsCnt =0 ; lie01.x = 0 ; lie01.speedX = 1; break; case "tru02": tru02.visible = false ; tru02.tripsCnt =0 ; tru02.x = -60; this.speedX = 0; lie02.visible=true ; lie02.tripsCnt =0 ; lie02.x = 0 ; lie02.speedX = 1; break; case "tru03": tru03.visible = false ; tru03.tripsCnt =0 ; tru03.x = -60; this.speedX = 0; lie03.visible=true ; lie03.tripsCnt =0 ; lie03.x = 0 ; lie03.speedX = 1; break; case "tru04": tru04.visible = false ; tru04.tripsCnt =0 ; tru04.x = -60; this.speedX = 0; lie04.visible=true ; lie04.tripsCnt =0 ; lie04.x = 0 ; lie04.speedX = 1; break; } playsIt = My_audio_Urls.indexOf("sounds/oops.mp3"); oopsScore ++; playAudio(cached_Audio_files[playsIt]); } if(yeslie){ switch(this.idTag) { case "lie01": lie01.visible=false ; lie01.tripsCnt =0 ; lie01.x = -60; this.speedX = 0; tru01.visible=true ; tru01.tripsCnt =0 ; tru01.x = 0; tru01.speedX = 1; break ; case "lie02": lie02.visible=false ; lie02.tripsCnt =0 ; lie02.x = -60; this.speedX = 0; tru02.visible=true ; tru02.tripsCnt =0 ; tru02.x = 0; tru02.speedX = 1; break ; case "lie03": lie03.visible=false ; lie03.tripsCnt =0 ; lie03.x = -60; this.speedX = 0; tru03.visible=true ; tru03.tripsCnt =0 ; tru03.x = 0; tru03.speedX = 1; break ; case "lie04": lie04.visible=false ; lie04.tripsCnt =0 ; lie04.x = -60; this.speedX = 0; tru04.visible=true ; tru04.tripsCnt =0 ; tru04.x = 0; tru04.speedX = 1; break ; } playsIt = My_audio_Urls.indexOf("sounds/CowMooSplat.mp3"); goodHits ++ ; playAudio(cached_Audio_files[playsIt]); } cowpies.splice(thisPie,1); //alert("cowpies.length= "+ cowpies.length); totalScore = ((goodHits * 4) - (oopsScore * 2)); } } } } } } I've fixed the indentation halfway just so I could understand what it was doing. There are many issues with this: 1. This assignment is completely useless: thisPie = i;, but if you changed it to thisPie = cowpies; it would improve efficiency by a lot. 2. String operations are expensive. The whole "idTag" property is unnecessary. thisIdTag = this.idTag ; // Another unnecessary assignment var findsIt = 0 ; var playsIt = 0 ; var yestru = thisIdTag.startsWith("tru"); // String operations var yeslie = thisIdTag.startsWith("lie"); // String operations if(yestru) { // Some code } if(yeslie){ // Some code } If you had given a boolean property isTrue to your TargetImages object you could use this code instead: if(this.isTrue) { // Some code } else { // Some code } 3. Your objects are aware of other objects' existence. This block of code has the object trying to identify itself among the others. Logic like this should not be inside the object's own code: if(this.idTag == "tru02") {this.speedX = 0; tru02.tripsCnt =0; this.visible=false ; lie02.speedX=2 ; lie02.visible=true ; // idTag = "lie02"; } if(this.idTag == "tru03") {this.speedX = 0; tru03.tripsCnt =0; this.visible=false ; lie03.speedX=1 ; lie03.visible=true ; // idTag = "lie03"; } if(this.idTag == "tru04") {this.speedX = 0; tru03.tripsCnt =0; this.visible=false ; lie04.speedX=1 ; lie04.visible=true ; // idTag = "lie03"; } if(this.idTag == "lie01") {this.speedX = 0; lie01.tripsCnt =0; this.visible=false ; tru01.speedX=1 ; tru01.visible=true ; // idTag = "tru01" ; } if(this.idTag == "lie02") {this.speedX = 0; lie02.tripsCnt =0; this.visible=false ; tru02.speedX=1 ; tru02.visible=true ; // idTag = "tru02" ; } if(this.idTag == "lie03") {this.speedX = 0; lie03.tripsCnt =0; tru03.speedX=1 ; tru03.visible=true ; // idTag = "tru03" ; } if(this.idTag == "lie04") {this.speedX = 0; lie04.tripsCnt =0; tru04.speedX=1 ; tru04.visible=true ; // idTag = "tru04" ; } The same thing is going on in these blocks of code: switch(this.idTag) { case "tru01":tru01.visible = false ; tru01.tripsCnt =0 ; tru01.x = -60; this.speedX = 0; lie01.visible=true ; lie01.tripsCnt =0 ; lie01.x = 0 ; lie01.speedX = 1; break; case "tru02": tru02.visible = false ; tru02.tripsCnt =0 ; tru02.x = -60; this.speedX = 0; lie02.visible=true ; lie02.tripsCnt =0 ; lie02.x = 0 ; lie02.speedX = 1; break; case "tru03": tru03.visible = false ; tru03.tripsCnt =0 ; tru03.x = -60; this.speedX = 0; lie03.visible=true ; lie03.tripsCnt =0 ; lie03.x = 0 ; lie03.speedX = 1; break; case "tru04": tru04.visible = false ; tru04.tripsCnt =0 ; tru04.x = -60; this.speedX = 0; lie04.visible=true ; lie04.tripsCnt =0 ; lie04.x = 0 ; lie04.speedX = 1; break; } switch(this.idTag) { case "lie01": lie01.visible=false ; lie01.tripsCnt =0 ; lie01.x = -60; this.speedX = 0; tru01.visible=true ; tru01.tripsCnt =0 ; tru01.x = 0; tru01.speedX = 1; break ; case "lie02": lie02.visible=false ; lie02.tripsCnt =0 ; lie02.x = -60; this.speedX = 0; tru02.visible=true ; tru02.tripsCnt =0 ; tru02.x = 0; tru02.speedX = 1; break ; case "lie03": lie03.visible=false ; lie03.tripsCnt =0 ; lie03.x = -60; this.speedX = 0; tru03.visible=true ; tru03.tripsCnt =0 ; tru03.x = 0; tru03.speedX = 1; break ; case "lie04": lie04.visible=false ; lie04.tripsCnt =0 ; lie04.x = -60; this.speedX = 0; tru04.visible=true ; tru04.tripsCnt =0 ; tru04.x = 0; tru04.speedX = 1; break ; } These are also string comparisons. Just to test each of the cases the computer is actually performing up to five operations per case, as opposed to a single operation used in boolean and integer comparisons. You should actually have an array of TargetImages() objects rather than having a variable for each one. Code like this is very repetitive and inflexible: allImgsCnt = 1; if(tru01.visible) { tru01.speedX = 1; tru01.update();} allImgsCnt = 2; if(tru02.visible) { tru02.speedX = 1; tru02.update(); } allImgsCnt = 3; if(tru03.visible) { tru03.speedX = 1; tru03.update(); } allImgsCnt = 4; if(tru04.visible) { tru04.speedX = 1; tru04.update(); } allImgsCnt = 5; if(lie01.visible) { lie01.speedX = 1; lie01.update(); } allImgsCnt = 6; if(lie02.visible) { lie02.speedX = 1; lie02.update(); } allImgsCnt = 7; if(lie03.visible) { lie03.speedX = 1; lie03.update(); } allImgsCnt = 8; if(lie04.visible) { lie04.speedX = 1; lie04.update(); } allImgsCnt = 0; if(thrower.moveMe) { thrower.update(); drawThrower(); } Link to comment Share on other sites More sharing options...
vmars316 Posted June 8, 2016 Author Share Posted June 8, 2016 Thanks Ingolme I see I have lots more work to do . Should keep me busy for a while . Thanks again . Link to comment Share on other sites More sharing options...
vmars316 Posted June 10, 2016 Author Share Posted June 10, 2016 Hello & Thanks , Can someone aim me at a w3schools how to "requestAnimationFrame()" ? I can't seem to find one . Thanks Link to comment Share on other sites More sharing options...
Ingolme Posted June 10, 2016 Share Posted June 10, 2016 W3Schools doesn't have a tutorial for it. You can find information about it on the Mozilla Developer Network: https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame The difference between requestAnimationFrame() and setInterval/setTimeout is that requestAnimationFrame() has a variable interval, so you need to use the timestamp provided in the function to calculate values. Link to comment Share on other sites More sharing options...
vmars316 Posted June 12, 2016 Author Share Posted June 12, 2016 W3Schools doesn't have a tutorial for it. You can find information about it on the Mozilla Developer Network: https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame The difference between requestAnimationFrame() and setInterval/setTimeout is that requestAnimationFrame() has a variable interval, so you need to use the timestamp provided in the function to calculate values. Thanks , In the info above , they mention a 'timestamp' . Is timestamp absolutely necessary . Here http://bencentra.com/canvas/canvas2.html He has an example without any timestamp . Is there a default value involved ? <!DOCTYPE HTML> <html> <head> <style> * { font-family: Calibri, Arial, sans-serif; } .center { text-align: center; } #mycanvas { display: block; width: 600px; height: 400px; margin: auto; border: 1px solid black; } button { font-size: 1em; } .pre { font-family: monospace; } #frames { font-weight: bold; } </style> </head> <body> <h1 class="center">Animation! Yeah!</h1> <p class="center">Playing around with <span class="pre">requestAnimationFrame();</span></p> <div id="buttons" class="center"> <button type="button" onclick="start();">Start</button> <button type="button" onclick="stop();">Stop</button> <button type="button" onclick="erase();">Erase</button> </div> <p class="center">Erase on mouseover: <input type="checkbox" id="check"/></p> <canvas id="mycanvas" width="600" height="400">Sorry, bro.</canvas> <p class="center"><span id="frames">0</span> frames have been rendered.</p> <script> // Handles to DOM elements var f = document.getElementById("frames"); var ch = document.getElementById("check"); // Set up the canvas var c = document.getElementById("mycanvas"); var ctx = c.getContext("2d"); var gid; var frameCount = 0; var iRed = 255; var iGreen = 0; var iBlue = 0; var inc = 4; // Mouse over to erase the canvas c.addEventListener("mousemove", function (e) { if (ch.checked) { var rect = c.getBoundingClientRect(); ctx.fillStyle = "#FFFFFF"; ctx.beginPath(); ctx.arc(e.clientX - rect.left, e.clientY - rect.top, 35, 0, 360, false); ctx.fill(); } }); // Clear the canvas function erase() { ctx.fillStyle = "#FFFFFF"; ctx.fillRect(0, 0, 600, 400); } // Run a frame of animation function animate() { var r = Math.round(Math.random() * 20) + 5; var x = Math.round(Math.random() * c.width); var y = Math.round(Math.random() * c.height); //setRandomColor(); incrementColor(); ctx.beginPath(); ctx.arc(x,y,r, 0, 360, false); ctx.fill(); gid = requestAnimationFrame(animate); f.innerHTML = frameCount++; } function setRandomColor() { var red = Math.floor(Math.random() * 255); var green = Math.floor(Math.random() * 255); var blue = Math.floor(Math.random() * 255); ctx.fillStyle = "rgb("+red+","+green+","+blue+")"; } function incrementColor() { if (iRed > 250 && iGreen <= 250 && iBlue <= 5) { iGreen += inc; } else if (iRed > 5 && iGreen > 250 && iBlue <= 5) { iRed =- inc; } else if (iRed <= 5 && iGreen > 250 && iBlue <= 250) { iBlue += inc; } else if (iRed <= 5 && iGreen > 5 && iBlue > 250) { iGreen -= inc; } else if (iRed <= 250 && iGreen <= 5 && iBlue > 250) { iRed += inc; } else { iBlue -= inc; } ctx.fillStyle = "rgb("+iRed+","+iGreen+","+iBlue+")"; } // Start animation function start() { console.log("animating"); animate(); } // Stop animation function stop() { console.log("stopping"); cancelAnimationFrame(gid); } </script> </body> </html> Link to comment Share on other sites More sharing options...
Ingolme Posted June 12, 2016 Share Posted June 12, 2016 The timstamp is passed as a parameter to your animate() function. function animate(TIMESTAMP) { console.log(TIMESTAMP); Link to comment Share on other sites More sharing options...
vmars316 Posted June 15, 2016 Author Share Posted June 15, 2016 (edited) The timstamp is passed as a parameter to your animate() function. function animate(TIMESTAMP) { console.log(TIMESTAMP); Thanks , but I don't understand : Is 'TIMESTAMP' a js reserved word or function . I am having trouble to piece it all together . I decided to make rAF a separate issue : And then come back to the other code problems you mentioned above . Thanks Edited June 15, 2016 by vmars316 Link to comment Share on other sites More sharing options...
vmars316 Posted June 28, 2016 Author Share Posted June 28, 2016 Foxy Mod ; I still working on your suggestions: You have quite a lot of messy code. Just replace this: var TargetImages = (function () { function TargetImages() { // Big block of code } // endof function TargetImages() return TargetImages;})(); // endof var TargetImages = (function () With this: function TargetImages() {// Big block of code} It's more readable and more efficient. I am hung up on how to turn tru01 into a function: truth01 = new TargetImages(); truth01.idTag = "tru01"; truth01.x = 0; truth01.y = 40; truth01.width = 64; truth01.height = 64; truth01.speedX = 2; truth01.speedY = 0; truth01.tripsCnt = 0; truth01.tripsMax = 2; truth01.visible = true; truth01.directionX = 1; // var TargetImages = (function() { function TargetImages() { this.nameTag = 'tru01' this.x = 0; this.y = 0; this.width = 32; this.height = 32; this.speedX = 4; this.speedY = 0; this.tripsCnt = 0; this.tripsMax = 2; this.visible = true; this.directionX = 1; this.update = function() { ctx.drawImage(targetImgs[targetImgsCnt], this.x, this.y, this.width, this.height); this.x = this.x + (this.speedX * this.directionX); // always = +1 or -1 idTag = this.idTag; if (this.idTag == "truth01") { this.speedX = 0; truth01.tripsCnt = 0; lies01.speedX = 3; idTag = "lies01"; this.visible = false; lies01.visible = true; } } } } return TargetImages; })(); I tried a few things , like this: var tru01 = new TargetImages(); var self = this; tru01.idTag = "tru01"; tru01.x = 128; tru01.y = 24; tru01.visible = false ; tru01.tripsCnt =0; tru01.tripsMax=0; // But I keep getting this type of error: Uncaught TypeError: tru01.update is not a function I need some help . Thanks Link to comment Share on other sites More sharing options...
Ingolme Posted June 28, 2016 Share Posted June 28, 2016 This would work. You don't need to wrap it in a closure and assign it to a variable. truth01 = new TargetImages(); truth01.idTag = "tru01"; truth01.x = 0; truth01.y = 40; truth01.width = 64; truth01.height = 64; truth01.speedX = 2; truth01.speedY = 0; truth01.tripsCnt = 0; truth01.tripsMax = 2; truth01.visible = true; truth01.directionX = 1; // function TargetImages() { this.nameTag = 'tru01' this.x = 0; this.y = 0; this.width = 32; this.height = 32; this.speedX = 4; this.speedY = 0; this.tripsCnt = 0; this.tripsMax = 2; this.visible = true; this.directionX = 1; this.update = function() { ctx.drawImage(targetImgs[targetImgsCnt], this.x, this.y, this.width, this.height); this.x = this.x + (this.speedX * this.directionX); // always = +1 or -1 idTag = this.idTag; if (this.idTag == "truth01") { this.speedX = 0; truth01.tripsCnt = 0; lies01.speedX = 3; idTag = "lies01"; this.visible = false; lies01.visible = true; } } } Link to comment Share on other sites More sharing options...
vmars316 Posted June 28, 2016 Author Share Posted June 28, 2016 Ah , works great , Thanks ! Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now