patternjavascriptModerate
"Do I understand the gravity of the situation?"
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:
Even though it's more simulation than game, there are some aspects of user interaction besides fiddling with the parameters:
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'
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
Robustness
-
Too much duplicate code, related to your not wanting to implement MVC, look at the code in
Functions are first class citizens
This
looks so much better than
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,
is probably faster than
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
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.
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
Crgoes 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 callresetGravityFieldor must face a bit of debugging time. LetrecalculateGravityFieldcallresetGravityFieldand 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.