Skip to content

JavaScript: Fixes to comparisons, iteration #3022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 17, 2019
Merged

JavaScript: Fixes to comparisons, iteration #3022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe1615a
js: Use === and !== with bool/numbers
ferdnyc Sep 27, 2019
9bf8728
controllers.js: replace indexed loops with for...of
ferdnyc Oct 20, 2019
7ff7c95
functions.js/track.js: Use for...of loops
ferdnyc Oct 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/timeline/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,11 @@ var App = angular.module('openshot-timeline', ['ui.bootstrap','ngAnimate']);

// Bind to keydown event (to detect SHIFT)
$( "body" ).keydown(function(event) {
if (event.which==16)
if (event.which === 16)
$('body').scope().shift_pressed = true;
});
$( "body" ).keyup(function(event) {
if ($('body').scope().shift_pressed)
$('body').scope().shift_pressed = false;
});
});

168 changes: 74 additions & 94 deletions src/timeline/js/controllers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,20 +320,18 @@ App.controller('TimelineCtrl',function($scope) {
}
// Determine if this property is a Keyframe
if (typeof object[child] == "object" && "Points" in object[child]) {
for (var point = 0; point < object[child].Points.length; point++) {
var co = object[child].Points[point].co;
if (co.X >= clip_start_x && co.X <= clip_end_x)
for (const point of object[child].Points) {
if (point.co.X >= clip_start_x && point.co.X <= clip_end_x)
// Only add keyframe coordinates that are within the bounds of the clip
keyframes[co.X] = co.Y;
keyframes[point.co.X] = point.co.Y;
}
}
// Determine if this property is a Color Keyframe
if (typeof object[child] == "object" && "red" in object[child]) {
for (var point = 0; point < object[child]["red"].Points.length; point++) {
var co = object[child]["red"].Points[point].co;
if (co.X >= clip_start_x && co.X <= clip_end_x)
for (const point of object[child]["red"].Points) {
if (point.co.X >= clip_start_x && point.co.X <= clip_end_x)
// Only add keyframe coordinates that are within the bounds of the clip
keyframes[co.X] = co.Y;
keyframes[point.co.X] = point.co.Y;
}
}
}
Expand All @@ -348,20 +346,18 @@ App.controller('TimelineCtrl',function($scope) {
}
// Determine if this property is a Keyframe
if (typeof object["effects"][effect][child] == "object" && "Points" in object["effects"][effect][child]) {
for (var point = 0; point < object["effects"][effect][child].Points.length; point++) {
var co = object["effects"][effect][child].Points[point].co;
if (co.X >= clip_start_x && co.X <= clip_end_x)
for (const point of object["effects"][effect][child].Points) {
if (point.co.X >= clip_start_x && point.co.X <= clip_end_x)
// Only add keyframe coordinates that are within the bounds of the clip
keyframes[co.X] = co.Y;
keyframes[point.co.X] = point.co.Y;
}
}
// Determine if this property is a Color Keyframe
if (typeof object["effects"][effect][child] == "object" && "red" in object["effects"][effect][child]) {
for (var point = 0; point < object["effects"][effect][child]["red"].Points.length; point++) {
var co = object["effects"][effect][child]["red"].Points[point].co;
if (co.X >= clip_start_x && co.X <= clip_end_x)
for (const point of object["effects"][effect][child]["red"].Points) {
if (point.co.X >= clip_start_x && point.co.X <= clip_end_x)
// Only add keyframe coordinates that are within the bounds of the clip
keyframes[co.X] = co.Y;
keyframes[point.co.X] = point.co.Y;
}
}
}
Expand Down Expand Up @@ -397,7 +393,7 @@ App.controller('TimelineCtrl',function($scope) {

// Determine actual x coordinate (over timeline)
var center_x = Math.max(cursor_x - track_labels_width, 0);
if (cursor_x == 0) {
if (cursor_x === 0) {
center_x = 0;
}

Expand Down Expand Up @@ -425,12 +421,12 @@ App.controller('TimelineCtrl',function($scope) {
// Set the audio data for a clip
$scope.setAudioData = function(clip_id, audio_data) {
// Find matching clip
for (var clip_index = 0; clip_index < $scope.project.clips.length; clip_index++) {
if ($scope.project.clips[clip_index].id == clip_id) {
for (const clip of $scope.project.clips) {
if (clip.id == clip_id) {
// Set audio data
$scope.$apply(function(){
$scope.project.clips[clip_index].audio_data = audio_data;
$scope.project.clips[clip_index].show_audio = true;
clip.audio_data = audio_data;
clip.show_audio = true;
});
Comment on lines 423 to 430
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned that for...of often makes the code cleaner: This change makes for a good representative example. The updated version is so much more readable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

timeline.qt_log("Audio data successful set on clip JSON");
break;
Expand All @@ -444,12 +440,12 @@ App.controller('TimelineCtrl',function($scope) {
// Hide the audio waveform for a clip
$scope.hideAudioData = function(clip_id, audio_data) {
// Find matching clip
for (var clip_index = 0; clip_index < $scope.project.clips.length; clip_index++) {
if ($scope.project.clips[clip_index].id == clip_id) {
for (const clip of $scope.project.clips) {
if (clip.id == clip_id) {
// Set audio data
$scope.$apply(function(){
$scope.project.clips[clip_index].show_audio = false;
$scope.project.clips[clip_index].audio_data = [];
clip.show_audio = false;
clip.audio_data = [];
});
break;
}
Expand All @@ -459,19 +455,19 @@ App.controller('TimelineCtrl',function($scope) {
// Redraw all audio waveforms on the timeline (if any)
$scope.reDrawAllAudioData = function() {
// Find matching clip
for (var clip_index = 0; clip_index < $scope.project.clips.length; clip_index++) {
if ("audio_data" in $scope.project.clips[clip_index] && $scope.project.clips[clip_index].audio_data.length > 0) {
for (const clip of $scope.project.clips) {
if ("audio_data" in clip && clip.audio_data.length > 0) {
// Redraw audio data (since it has audio data)
drawAudio($scope, $scope.project.clips[clip_index].id);
drawAudio($scope, clip.id);
}
}
};

// Does clip have audio_data?
$scope.hasAudioData = function(clip_id) {
// Find matching clip
for (var clip_index = 0; clip_index < $scope.project.clips.length; clip_index++) {
if ($scope.project.clips[clip_index].id == clip_id && "audio_data" in $scope.project.clips[clip_index] && $scope.project.clips[clip_index].audio_data.length > 0) {
for (const clip of $scope.project.clips) {
if (clip.id == clip_id && "audio_data" in clip && clip.audio_data.length > 0) {
return true;
break;
}
Expand Down Expand Up @@ -581,11 +577,11 @@ App.controller('TimelineCtrl',function($scope) {

// Update scope
$scope.$apply(function() {
for (var clip_index = 0; clip_index < $scope.project.clips.length; clip_index++) {
$scope.project.clips[clip_index].selected = false;
for (const clip of $scope.project.clips) {
clip.selected = false;
}
for (var effect_index = 0; effect_index < $scope.project.effects.length; effect_index++) {
$scope.project.effects[effect_index].selected = false;
for (const effect of $scope.project.effects) {
effect.selected = false;
}
});
};
Expand All @@ -594,14 +590,14 @@ App.controller('TimelineCtrl',function($scope) {
$scope.SelectAll = function() {
$scope.$apply(function() {
// Select all clips
for (var clip_index = 0; clip_index < $scope.project.clips.length; clip_index++) {
$scope.project.clips[clip_index].selected = true;
timeline.addSelection($scope.project.clips[clip_index].id, "clip", false);
for (const clip of $scope.project.clips) {
clip.selected = true;
timeline.addSelection(clip.id, "clip", false);
}
// Select all transitions
for (var effect_index = 0; effect_index < $scope.project.effects.length; effect_index++) {
$scope.project.effects[effect_index].selected = true;
timeline.addSelection($scope.project.effects[effect_index].id, "transition", false);
for (const effect of $scope.project.effects) {
effect.selected = true;
timeline.addSelection(effect.id, "transition", false);
}
});
};
Expand Down Expand Up @@ -632,17 +628,17 @@ App.controller('TimelineCtrl',function($scope) {
}

// Unselect all clips
for (var clip_index = 0; clip_index < $scope.project.clips.length; clip_index++) {
if ($scope.project.clips[clip_index].id == id) {
$scope.project.clips[clip_index].selected = true;
for (const clip of $scope.project.clips) {
if (clip.id == id) {
clip.selected = true;
if ($scope.Qt) {
timeline.addSelection(id, "clip", clear_selections);
}
}
else if (clear_selections && !is_ctrl) {
$scope.project.clips[clip_index].selected = false;
clip.selected = false;
if ($scope.Qt) {
timeline.removeSelection($scope.project.clips[clip_index].id, "clip");
timeline.removeSelection(clip.id, "clip");
}
}
}
Expand Down Expand Up @@ -675,16 +671,16 @@ App.controller('TimelineCtrl',function($scope) {
}

// Unselect all transitions
for (var tran_index = 0; tran_index < $scope.project.effects.length; tran_index++) {
if ($scope.project.effects[tran_index].id == id) {
$scope.project.effects[tran_index].selected = true;
for (const tran of $scope.project.effects) {
if (tran.id == id) {
tran.selected = true;
if ($scope.Qt)
timeline.addSelection(id, "transition", clear_selections);
}
else if (clear_selections && !is_ctrl) {
$scope.project.effects[tran_index].selected = false;
tran.selected = false;
if ($scope.Qt)
timeline.removeSelection($scope.project.effects[tran_index].id, "transition");
timeline.removeSelection(tran.id, "transition");
}
}
};
Expand All @@ -710,8 +706,7 @@ App.controller('TimelineCtrl',function($scope) {
$scope.ResizeTimeline = function() {
// Unselect all clips
var furthest_right_edge = 0;
for (var clip_index = 0; clip_index < $scope.project.clips.length; clip_index++) {
var clip = $scope.project.clips[clip_index];
for (const clip of $scope.project.clips) {
var right_edge = clip.position + (clip.end - clip.start);
if (right_edge > furthest_right_edge)
furthest_right_edge = right_edge;
Expand Down Expand Up @@ -901,20 +896,20 @@ $scope.SetTrackLabel = function (label) {
// Select new clip object (and unselect others)
// This needs to be done inline due to async issues with the
// above calls to SelectClip/SelectTransition
for (var clip_index = 0; clip_index < $scope.project.clips.length; clip_index++) {
if ($scope.project.clips[clip_index].id == item_id) {
$scope.project.clips[clip_index].selected = true;
for (const clip of $scope.project.clips) {
if (clip.id == item_id) {
clip.selected = true;
} else {
$scope.project.clips[clip_index].selected = false;
clip.selected = false;
}
}

// Select new transition object (and unselect others)
for (var tran_index = 0; tran_index < $scope.project.effects.length; tran_index++) {
if ($scope.project.effects[tran_index].id == item_id) {
$scope.project.effects[tran_index].selected = true;
for (const tran of $scope.project.effects) {
if (tran.id == item_id) {
tran.selected = true;
} else {
$scope.project.effects[tran_index].selected = false;
tran.selected = false;
}
}

Expand Down Expand Up @@ -972,12 +967,10 @@ $scope.SetTrackLabel = function (label) {
}
// Loop through each layer (looking for the closest track based on Y coordinate)
bounding_box.track_position = 0;
for (var layer_index = $scope.project.layers.length - 1; layer_index >= 0 ; layer_index--) {
var layer = $scope.project.layers[layer_index];

for (const layer of $scope.project.layers) {
// Compare position of track to Y param (for unlocked tracks)
if (!layer.lock) {
if ((top < layer.y && top > bounding_box.track_position) || bounding_box.track_position==0) {
if ((top < layer.y && top > bounding_box.track_position) || bounding_box.track_position === 0) {
// return first matching layer
bounding_box.track_position = layer.y;
}
Expand All @@ -1004,9 +997,7 @@ $scope.SetTrackLabel = function (label) {

$scope.$apply(function() {
// Loop through each layer
for (var layer_index = 0; layer_index < $scope.project.layers.length; layer_index++) {
var layer = $scope.project.layers[layer_index];

for (const layer of $scope.project.layers) {
// Find element on screen (bound to this layer)
var layer_elem = $("#track_" + layer.number);
if (layer_elem) {
Expand Down Expand Up @@ -1075,9 +1066,7 @@ $scope.SetTrackLabel = function (label) {
var original_right = original_clip.position + (original_clip.end - original_clip.start);

// Search through all other clips on this track, and look for overlapping ones
for (var index = 0; index < $scope.project.clips.length; index++) {
var clip = $scope.project.clips[index];

for (const clip of $scope.project.clips) {
// skip clips that are not on the same layer
if (original_clip.layer != clip.layer) {
continue;
Expand Down Expand Up @@ -1105,9 +1094,7 @@ $scope.SetTrackLabel = function (label) {
}
// Search through all existing transitions, and don't overlap an existing one
if (transition_size != null) {
for (var tran_index = 0; tran_index < $scope.project.effects.length; tran_index++) {
var tran = $scope.project.effects[tran_index];

for (const tran of $scope.project.effects) {
// skip transitions that are not on the same layer
if (tran.layer != transition_size.layer) {
continue;
Expand Down Expand Up @@ -1140,12 +1127,9 @@ $scope.SetTrackLabel = function (label) {
var diffs = [];

// Loop through each pixel position (supports multiple positions: i.e. left and right side of bounding box)
for (var pos_index = 0; pos_index < pixel_positions.length; pos_index++) {
var position = pixel_positions[pos_index];

for (const position of pixel_positions) {
// Add clip positions to array
for (var index = 0; index < $scope.project.clips.length; index++) {
var clip = $scope.project.clips[index];
for (const clip of $scope.project.clips) {
var clip_left_position = clip.position * $scope.pixelsPerSecond;
var clip_right_position = (clip.position + (clip.end - clip.start)) * $scope.pixelsPerSecond;

Expand All @@ -1159,8 +1143,7 @@ $scope.SetTrackLabel = function (label) {
}

// Add transition positions to array
for (var index = 0; index < $scope.project.effects.length; index++) {
var transition = $scope.project.effects[index];
for (const transition of $scope.project.effects) {
var tran_left_position = transition.position * $scope.pixelsPerSecond;
var tran_right_position = (transition.position + (transition.end - transition.start)) * $scope.pixelsPerSecond;

Expand All @@ -1169,14 +1152,13 @@ $scope.SetTrackLabel = function (label) {
if (ignore_ids.hasOwnProperty(transition.id)) {
continue;
}

diffs.push({'diff' : position - tran_left_position, 'position' : tran_left_position}, // left side of transition
{'diff' : position - tran_right_position, 'position' : tran_right_position}); // right side of transition
}

// Add marker positions to array
for (var index = 0; index < $scope.project.markers.length; index++) {
var marker = $scope.project.markers[index];
for (const marker of $scope.project.markers) {
var marker_position = marker.position * $scope.pixelsPerSecond;

diffs.push({'diff' : position - marker_position, 'position' : marker_position}, // left side of marker
Expand All @@ -1187,25 +1169,25 @@ $scope.SetTrackLabel = function (label) {
var playhead_pixel_position = $scope.project.playhead_position * $scope.pixelsPerSecond;
var playhead_diff = position - playhead_pixel_position;
diffs.push({'diff' : playhead_diff, 'position' : playhead_pixel_position });

// Loop through diffs (and find the smallest one)
for (var diff_index = 0; diff_index < diffs.length; diff_index++) {
var diff = diffs[diff_index].diff;
var position = diffs[diff_index].position;
for (const each_diff of diffs) {
var diff = each_diff.diff;
var diff_position = each_diff.position;
var abs_diff = Math.abs(diff);

// Check if this clip is nearby
if (abs_diff < smallest_abs_diff && abs_diff <= threshold) {
// This one is smaller
smallest_diff = diff;
smallest_abs_diff = abs_diff;
snapping_position = position;
snapping_position = diff_position;
}
}
}

// no nearby found?
if (smallest_diff == 900.0) {
if (smallest_diff === 900.0) {
smallest_diff = 0.0;
}

Expand Down Expand Up @@ -1294,9 +1276,7 @@ $scope.SetTrackLabel = function (label) {
$scope.ApplyJsonDiff = function(jsonDiff) {

// Loop through each UpdateAction
for (var action_index = 0; action_index < jsonDiff.length; action_index++) {
var action = jsonDiff[action_index];

for (const action of jsonDiff) {
// Iterate through the key levels (looking for a matching element in the $scope.project)
var previous_object = null;
var current_object = $scope.project;
Expand Down Expand Up @@ -1440,7 +1420,7 @@ $scope.SetTrackLabel = function (label) {
// ############ DEBUG STUFFS ################## //

$scope.ToggleDebug = function() {
if ($scope.debug == true) {
if ($scope.debug === true) {
$scope.debug = false;
}
else {
Expand Down
Loading