Pls critique my code!

Hi. I’m having a lot of problems with things not working the way they should, and things not working the way other people say they should. Can you please critique my code? I’m new to coding and just hacking things together. I think some of my code is redundant and poorly written (use of let vs var?/do i need to load the files twice?/are things in the right order?/could I condense functions anywhere?/should my image and sound be listed in both setup and draw as they are?) and I’m guessing it’s causing me some inflexibility as I try to work in new code to this project:

let img;
let song;

function windowResized() 
{
  resizeCanvas(windowWidth, windowHeight);
}


function preload()
{
img =  loadImage('assets/picture.png');
song = loadSound('assets/recording.m4a');
}

function setup() 
{
var cnv = createCanvas(windowWidth, windowHeight);
cnv.style('display', 'block');

imageMode(CENTER);

img =  loadImage('assets/picture.png');
song = loadSound('assets/recording.m4a');

frameRate(12);
}

function mousePressed() 
  {
  getAudioContext().resume() 
  song.play();
  }
  
function mouseMoved() 
  {
  song.play();
  }
  
function touchStarted() 
  {
  getAudioContext().resume() 
  song.play();
  }
  
function touchMoved() 
  {
  song.play();
  }
  
  
function draw() 
{
  var mX=mouseX
  var mY=mouseY

image(img, mX, mY, img.width / 2,     
  img.height / 2);
}


Not at all! Callback preload() is the single best place to load assets. :star_struck:

BtW, you’ve got some missing indentation on your posted code. :woozy_face:

We can use Beautifier.io in order to clean up our JS code. :wink:

Here’s my own take on your sketch (Warning: Not fully tested!): :sunglasses:


index.html:

<!DOCTYPE html>

<meta charset=utf-8>
<meta name=viewport content=width=device-width,initial-scale=1>

<script defer src=https://cdn.JsDelivr.net/npm/p5></script>
<script defer src=https://Unpkg.com/p5/lib/addons/p5.dom.min.js></script>
<script defer src=https://Unpkg.com/p5/lib/addons/p5.sound.min.js></script>

<script defer src=sketch.js></script>

sketch.js:

// https://Discourse.Processing.org/t/pls-critique-my-code/12855/2
// @Epignosis567 & @GoToLoop (2019-Jul-21)

'use strict';

const IMG_NAME = 'picture.png', SONG_NAME = 'recording.m4a',
      FOLDER = 'assets/', SUSPEND_STATE = 'suspended', FPS = 12;

var touchStarted = mousePressed, touchMoved = mouseMoved;

let ori, img, song;

function preload() {
  ori = loadImage(FOLDER + IMG_NAME);
  song = loadSound(FOLDER + SONG_NAME);
}

function setup() {
  windowResized();
  _renderer.style('display', 'block');
  frameRate(FPS).imageMode(CENTER);
}

function draw() {
  clear().image(img, mouseX, mouseY);
}

function windowResized() {
  resizeCanvas(windowWidth, windowHeight, true);
  (img = ori.get()).resize(width >> 1, height >> 1);
}

function mousePressed() {
  const audCtx = getAudioContext();
  audCtx.state === SUSPEND_STATE && audCtx.resume();
  return mouseMoved();
}

function mouseMoved() {
  song.isPlaying() || song.play();
  return false;
}

1 Like

Oh wow, this is very different… Thanks so much this is enormously helpful!

Any idea why the image resolution and size malfunction on window resize? I’m not able to make my image size responsive at all, nothing I’ve tried works, but in your code it works except the resolution gets destroyed when the window is made smaller and the size gets distorted when it’s made large.

B/c I don’t have your asset files, I haven’t tested my version properly. :grimacing:

Maybe b/c the same p5.Image is resize() over & over. :thinking:
I’ve made some changes in my version. Check it out my reply above again. :innocent:

Now it keeps the original loaded image in global variable ori:
let ori, img, song;
ori = loadImage(FOLDER + IMG_NAME);

And it clones it via method p5.Image::get() inside windowResized():
p5js.org/reference/#/p5.Image/get

Stores it in global variable img; then invokes method p5.Image::resize() right afterwards:
p5js.org/reference/#/p5.Image/resize
(img = ori.get()).resize(width >> 1, height >> 1);

1 Like