HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

"Do I understand the gravity of the situation?"

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
situationunderstandthegravity

Problem

What is this?

This is a two-dimensional physics simulator that models gravity and collisions between circular objects (though it doesn't model rotation).
How do you use it?

You can tweak the following parameters of the simulation while it is running:

  • Number of objects: The specified number of gravitational objects will be added to the scene with random sizes and velocities (replacing any existing objects) upon clicking "Reset".



  • Delay: The number of milliseconds to intentionally delay the simulation between steps when it's running on automatic (the better to see what's happening). You can also advance the simulation manually, in which case the delay has no effect.



  • Border Behavior: Determines what happens to objects when they cross the borders of the scene. Options are Annihilate, Unbounded (default), Loop, Ricochet (which is perfectly elastic), and 50% Ricochet (which is less so).



  • Show Fabric of Spacetime: Toggles whether to display visual markers that indicate the strength of gravitational fields by their displacement.



  • Fabric Granularity: A number that determines the number of markers displayed (lower numbers result in higher marker density).



  • Handle Collisions: Toggles whether to perform collision calculations when object velocities would cause them to intersect.



  • Coefficient of Restitution: A number between 0 and 1 that determines the elasticity of collisions. At 1.0, collisions are perfectly elastic. At 0.0, collisions are perfectly inelastic and objects stick to each other upon collision.



Even though it's more simulation than game, there are some aspects of user interaction besides fiddling with the parameters:

  • Click to add an object.



  • Click and drag to add an object with an initial velocity.



  • Right click to remove an object.



What do I care about?
Improving Performance

Anything that can improve performance when no delay is specified.

Based on the massive performance improvements I saw when I refactored my implementation of Conway'

Solution

I spent quite some time looking at this:

High Level

I love what you did here, a great showcase of using JavaScript in a powerful way. But from a readability and maintainability perspective, this code is not very good.

Globals, globals everywhere

This is good reading. Your code is suffering from (1) and (2)

Naming

  • Your constants are not consistently named, I would advise to only use ALLCAPS for number and string constants, it just looks wrong to see CANVAS



  • I am all for Spartan coding, but Cr goes a bit too far



  • What is planting? (See comments)



  • plantx, planty, mouseX, mouseY,



  • let ox = o.x, oy = o.y;



Robustness

  • Imagine someone maintains this code, and wants to call recalculateGravityField. That person has to know to first call resetGravityField or must face a bit of debugging time. Let recalculateGravityField call resetGravityField and your code will be more robust.



-
Too much duplicate code, related to your not wanting to implement MVC, look at the code in dragCursor and ddlFabricGranularity.addEventListener share a ton of code, imagine having an ui.update()

ddlFabricGranularity.addEventListener("change", function ddlFabricGranularityChanged() {
  fadeMultiplier = this.value / FADE_RATE;
  recalculateGravityField(); //Now calls resetGravityField();
  ui.update();
});


Functions are first class citizens

This

document.getElementById("Reset").addEventListener("click", reset );


looks so much better than

document.getElementById("Reset").addEventListener("click", function() {
  reset();
});


Unnamed functions ruin debugging

You have a ton of anonymous functions, making the stack-trace harder to read, just name those functions.

Speed

Following your logic and general wisdom,

for (let i = 0; i < numObjects; i++) {
    let o = stellarObjects[i];
    applyObjectGravityToFabric(o.x, o.y, o.massEffect);
  }


is probably faster than

for (let i = 0; i < numObjects; i++) {
    let o = stellarObjects[i];
    let ox = o.x,
      oy = o.y,
      om = o.massEffect;
    applyObjectGravityToFabric(ox, oy, om);
  }


it might turn out to be the same due to optimization by the JS engine.

From a robustness perspective, I would just throw the whole object to applyObjectGravityToFabric and rework applyObjectGravityToFabric.

for (let i = 0; i < numObjects; i++) {
    applyObjectGravityToFabric(stellarObjects[i]);
  }


A plea for MVC

Read this: https://addyosmani.com/resources/essentialjsdesignpatterns/book/#detailmvc

And give it a shot, your code could become amazing AND readable.

Code Snippets

ddlFabricGranularity.addEventListener("change", function ddlFabricGranularityChanged() {
  fadeMultiplier = this.value / FADE_RATE;
  recalculateGravityField(); //Now calls resetGravityField();
  ui.update();
});
document.getElementById("Reset").addEventListener("click", reset );
document.getElementById("Reset").addEventListener("click", function() {
  reset();
});
for (let i = 0; i < numObjects; i++) {
    let o = stellarObjects[i];
    applyObjectGravityToFabric(o.x, o.y, o.massEffect);
  }
for (let i = 0; i < numObjects; i++) {
    let o = stellarObjects[i];
    let ox = o.x,
      oy = o.y,
      om = o.massEffect;
    applyObjectGravityToFabric(ox, oy, om);
  }

Context

StackExchange Code Review Q#152212, answer score: 10

Revisions (0)

No revisions yet.