Skip to content
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

Jump to bottom

DEL.X / DEL.R ops #164

Merged
merged 11 commits into from
Oct 15, 2018
Merged

DEL.X / DEL.R ops #164

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5296d2e
added RAT
alpha-cactus Oct 3, 2018
90cea46
fixed RAT error in op.c
alpha-cactus Oct 3, 2018
1f47cc7
updated xdel op
alpha-cactus Oct 10, 2018
71a9616
Merge remote-tracking branch 'upstream/master'
alpha-cactus Oct 10, 2018
e31d306
xdel op update and common delay code function
alpha-cactus Oct 10, 2018
b50f2d8
updated variable names, changed xdel op names
alpha-cactus Oct 12, 2018
f0bfd37
added info the CHANGELOG.md
alpha-cactus Oct 12, 2018
ec7aacf
added DEL.X and DEL.R documentation
alpha-cactus Oct 12, 2018
7beff19
added info the CHANGELOG.md
alpha-cactus Oct 12, 2018
1fd3574
added check to delay_common_add helper function in delay.c
alpha-cactus Oct 13, 2018
4151cdf
updated delay common add function to properly check free delay slots,…
alpha-cactus Oct 14, 2018
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
69 changes: 29 additions & 40 deletions src/ops/delay.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include "teletype.h"
#include "teletype_io.h"

static void delay_common_add(scene_state_t *ss, exec_state_t *es, int16_t i,
static bool delay_common_add(scene_state_t *ss, exec_state_t *es,
int16_t delay_time,
const tele_command_t *post_command);

Expand All @@ -30,34 +30,37 @@ const tele_mod_t mod_DEL_R = MAKE_MOD(DEL.R, mod_DEL_R_func, 2);

// common code to queue a delay shared between all delay ops
// NOTE it is the responsibility of the callee to call tele_has_delays
static void delay_common_add(scene_state_t *ss, exec_state_t *es, int16_t i,
static bool delay_common_add(scene_state_t *ss, exec_state_t *es,
int16_t delay_time,
const tele_command_t *post_command) {
int16_t i = 0;

// 0 is the magic number for an empty slot.
// Be careful not to set delay.time[i] to 0 before calling this function.
while (ss->delay.time[i] != 0 && i != DELAY_SIZE) i++;

if (delay_time < 1) delay_time = 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a check for the delay time to the common code function. removed the check from the DEL op function, but it's still in the DEL.X and DEL.R because the wrap check would cause 32ms delays if the delay time param was negative.


if (i < DELAY_SIZE) {
ss->delay.count++;
Copy link
Member

Choose a reason for hiding this comment

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

i would add a check here to make sure it doesn't exceed DELAY_SIZE - imho it's a good idea to have safety checks in helper functions.

Copy link
Contributor Author

@alpha-cactus alpha-cactus Oct 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

ah good catch I can add that in. on the same topic do you think it would be good to move tele_has_delays into the helper function as well? the downside is it would get called excess times times by DEL.X, but the change would make the helper function slightly more robust for other future usage.

Copy link
Member

Choose a reason for hiding this comment

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

i'd leave it out - the function has a well defined and logical contract (it adds a delay), tele_has_delays is outside of that (and only has meaning to where this function is called from).

ss->delay.time[i] = delay_time;
ss->delay.origin_script[i] = es_variables(es)->script_number;
ss->delay.origin_i[i] = es_variables(es)->i;
copy_command(&ss->delay.commands[i], post_command);

return true;
}

return false;
}

static void mod_DEL_func(scene_state_t *ss, exec_state_t *es,
command_state_t *cs,
const tele_command_t *post_command) {
int16_t i = 0;
int16_t delay_time = cs_pop(cs);

if (delay_time < 1) delay_time = 1;

// 0 is the magic number for an empty slot.
// Be careful not to set delay.time[i] to 0 before calling this function.
while (ss->delay.time[i] != 0 && i != DELAY_SIZE) i++;

if (i < DELAY_SIZE) {
delay_common_add(ss, es, i, delay_time, post_command);
tele_has_delays(ss->delay.count > 0);
}
delay_common_add(ss, es, delay_time, post_command);
tele_has_delays(ss->delay.count > 0);
}

static void op_DEL_CLR_get(const void *NOTUSED(data), scene_state_t *ss,
Expand All @@ -69,30 +72,23 @@ static void op_DEL_CLR_get(const void *NOTUSED(data), scene_state_t *ss,
static void mod_DEL_X_func(scene_state_t *ss, exec_state_t *es,
command_state_t *cs,
const tele_command_t *post_command) {
int16_t i = 0;
int16_t num_delays = cs_pop(cs);
int16_t delay_time = cs_pop(cs);
int16_t delay_time_next;

if (delay_time < 1) delay_time = 1; // minimum delay time = 1ms

delay_time_next = delay_time; // set first delay time to delay time

// 0 is the magic number for an empty slot.
// Be careful not to set delay.time[i] to 0 before calling this function.
while (ss->delay.time[i] != 0 && i != DELAY_SIZE) i++;
if (delay_time < 1) delay_time = 1;

while (i < DELAY_SIZE && num_delays > 0) {
// queue the delay
delay_common_add(ss, es, i, delay_time_next, post_command);
// set first delay time to delay time
delay_time_next = delay_time;

while ( num_delays > 0 && delay_common_add(ss, es, delay_time_next, post_command) )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the function call is part of the while check instead of using a separate bool variable. pretty sure this is fine, but I may be missing something.

{
// increment delay time for next delay
// normalise incremented value to stop negative wrap from increment
delay_time_next += delay_time;
delay_time_next = normalise_value(1, 32767, 1, delay_time_next);

num_delays--;
i++;
}

tele_has_delays(ss->delay.count > 0);
Expand All @@ -101,30 +97,23 @@ static void mod_DEL_X_func(scene_state_t *ss, exec_state_t *es,
static void mod_DEL_R_func(scene_state_t *ss, exec_state_t *es,
command_state_t *cs,
const tele_command_t *post_command) {
int16_t i = 0;
int16_t num_delays = cs_pop(cs); // number of chained delays
int16_t delay_time = cs_pop(cs); // delay time
int16_t delay_time_next; // incremented delay time

if (delay_time < 1) delay_time = 1; // minimum delay time = 1ms

delay_time_next = 1; // set first delay time to 1ms to trigger immediately
int16_t num_delays = cs_pop(cs);
int16_t delay_time = cs_pop(cs);
int16_t delay_time_next;

// 0 is the magic number for an empty slot.
// Be careful not to set delay.time[i] to 0 before calling this function.
while (ss->delay.time[i] != 0 && i != DELAY_SIZE) i++;
if (delay_time < 1) delay_time = 1;

while (i < DELAY_SIZE && num_delays > 0) {
// queue the delay
delay_common_add(ss, es, i, delay_time_next, post_command);
// set first delay time to 1ms to trigger immediately
delay_time_next = 1;

while ( num_delays > 0 && delay_common_add(ss, es, delay_time_next, post_command) )
{
// increment delay time for next delay
// normalise incremented value to stop negative wrap from increment
delay_time_next += delay_time;
delay_time_next = normalise_value(1, 32767, 1, delay_time_next);

num_delays--;
i++;
}

tele_has_delays(ss->delay.count > 0);
Expand Down