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.

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

Less temporary memory allocations & share AF data when possible #632

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 9 additions & 16 deletions src/formats/script.c
Original file line number Diff line number Diff line change
Expand Up @@ -203,20 +203,16 @@ static int read_int(const str *src, int *original) {
return 0; // If we found no number, just return default 0.
}

str test;
str_from_slice(&test, src, start, end);
if(!str_to_int(&test, &value)) {
if(!str_to_int(src, start, &value)) {
value = 0; // This should never really happen.
}
str_free(&test);
*original = end;
return value;
}

static bool test_tag_slice(const str *test, sd_script_tag *new, str *src, int *now) {
const int len = str_size(test);
const int jmp = *now + len;
if(sd_tag_info(str_c(test), &new->has_param, &new->key, &new->desc) == 0) {
static bool test_tag_slice(const char *test, int size, sd_script_tag *new, str *src, int *now) {
if(sd_tag_info(test + *now, size, &new->has_param, &new->key, &new->desc) == 0) {
const int jmp = *now + strlen(new->key);
// Ensure that tag has no value, if value is not desired.
if(!new->has_param && find_numeric_span(src, jmp) > jmp) {
return false;
Expand All @@ -238,19 +234,16 @@ static bool parse_tag(sd_script_tag *new, str *src, int *now) {
}

// Check if tag is legit.
str test;
for(int m = 3; m > 0; m--) {
str_from_slice(&test, src, *now, *now + m);
if(str_equal_c(&test, "usw") && find_numeric_span(src, *now + m) > *now + m) {
// str_from_slice(&test, src, *now, *now + m);
if(strncmp(src->data + *now, "usw", m) == 0 && find_numeric_span(src, *now + m) > *now + m) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(strncmp(src->data + *now, "usw", m) == 0 && find_numeric_span(src, *now + m) > *now + m) {
if(strncmp(str_c(src) + *now, "usw", 3) == 0 && find_numeric_span(src, *now + m) > *now + m) {

The strncmp call should compare all 3 characters like the str_equal_c did before, not just the first m.

// Fixup rare usw30/usw case, which can be u + sw30 or us + w
str_free(&test);
// str_free(&test);
return false;
}
if(test_tag_slice(&test, new, src, now)) {
str_free(&test);
if(test_tag_slice(src->data, m, new, src, now)) {
return true;
}
str_free(&test);
}
return false;
}
Expand Down Expand Up @@ -595,7 +588,7 @@ int sd_script_set_tag(sd_script *script, int frame_id, const char *tag, int valu

// Get tag information
sd_script_tag new;
if(sd_tag_info(tag, &new.has_param, &new.key, &new.desc) != SD_SUCCESS) {
if(sd_tag_info(tag, strlen(tag), &new.has_param, &new.key, &new.desc) != SD_SUCCESS) {
return SD_INVALID_INPUT;
}
if(new.has_param) {
Expand Down
2 changes: 1 addition & 1 deletion src/formats/taglist.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ extern const int sd_taglist_size; ///< Taglist size
* \param tag A pointer to the tag string in library memory. Will be ignored if set to NULL.
* \param desc A pointer to the description in library memory. Will be ignored if set to NULL.
*/
int sd_tag_info(const char *search_tag, int *req_param, const char **tag, const char **desc);
int sd_tag_info(const char *search_tag, int search_len, int *req_param, const char **tag, const char **desc);

#ifdef __cplusplus
}
Expand Down
4 changes: 2 additions & 2 deletions src/formats/taglist_helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
#include <stdlib.h>
#include <string.h>

int sd_tag_info(const char *search_tag, int *req_param, const char **tag, const char **desc) {
int sd_tag_info(const char *search_tag, int search_len, int *req_param, const char **tag, const char **desc) {
for(int i = 0; i < sd_taglist_size; i++) {
if(strcmp(search_tag, sd_taglist[i].tag) == 0) {
if(strlen(sd_taglist[i].tag) == search_len && strncmp(search_tag, sd_taglist[i].tag, search_len) == 0) {
if(req_param != NULL)
*req_param = sd_taglist[i].has_param;
if(tag != NULL)
Expand Down
33 changes: 23 additions & 10 deletions src/game/gui/textslider.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,35 +16,37 @@ typedef struct {
int *pos;
int has_off;
int positions;
char txt[20];

void *userdata;
textslider_slide_cb slide;
} textslider;

static void textslider_render(component *c) {
static void render_text(component *c) {
Comment on lines -24 to +25
Copy link
Contributor

@Nopey Nopey Nov 26, 2024

Choose a reason for hiding this comment

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

Please rename this method to something that does not contain the word "render."

textslider_format? textslider_updatestr?

EDIT: probably better to remove the textslider changes from this PR in favor of PR 773.

textslider *tb = widget_get_obj(c);
str txt;
str_from_format(&txt, "%s ", tb->text);
int off = snprintf(tb->txt, sizeof(tb->txt), "%s ", tb->text);
if(tb->has_off && *tb->pos == 0) {
str_append_c(&txt, "OFF");
snprintf(tb->txt + off, sizeof(tb->txt) - off, "%s", "OFF");
Copy link
Member

Choose a reason for hiding this comment

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

I dislike this -- the idea with str type is that we can later convert it to utf-8. This is taking steps backwards. How about we cache the str to textslider instead ?

} else {
for(int i = 0; i < tb->positions; i++) {
if(i + 1 > *tb->pos) {
str_append_c(&txt, "|");
off += snprintf(tb->txt + off, sizeof(tb->txt) - off, "%s", "|");
} else {
str_append_c(&txt, "\x7f");
off += snprintf(tb->txt + off, sizeof(tb->txt) - off, "%s", "\x7f");
}
}
}
}

static void textslider_render(component *c) {
textslider *tb = widget_get_obj(c);
text_mode mode = TEXT_UNSELECTED;
if(component_is_selected(c)) {
mode = TEXT_SELECTED;
} else if(component_is_disabled(c)) {
mode = TEXT_DISABLED;
}
text_render(&tb->tconf, mode, c->x, c->y, c->w, c->h, str_c(&txt));
str_free(&txt);
text_render(&tb->tconf, mode, c->x, c->y, c->w, c->h, tb->txt);
}

static int textslider_action(component *c, int action) {
Expand All @@ -60,6 +62,9 @@ static int textslider_action(component *c, int action) {
if(tb->slide) {
tb->slide(c, tb->userdata, *tb->pos);
}

render_text(c);

// reset ticks so text is bright
tb->ticks = 0;
tb->dir = 0;
Expand All @@ -75,6 +80,9 @@ static int textslider_action(component *c, int action) {
if(tb->slide) {
tb->slide(c, tb->userdata, *tb->pos);
}

render_text(c);

// reset ticks so text is bright
tb->ticks = 0;
tb->dir = 0;
Expand Down Expand Up @@ -122,6 +130,8 @@ component *textslider_create(const text_settings *tconf, const char *text, const
tb->slide = cb;
widget_set_obj(c, tb);

render_text(c);

widget_set_render_cb(c, textslider_render);
widget_set_action_cb(c, textslider_action);
widget_set_tick_cb(c, textslider_tick);
Expand All @@ -133,7 +143,10 @@ component *textslider_create_bind(const text_settings *tconf, const char *text,
unsigned int positions, int has_off, textslider_slide_cb cb, void *userdata,
int *bind) {
component *c = textslider_create(tconf, text, help, positions, has_off, cb, userdata);
textslider *ts = widget_get_obj(c);
ts->pos = (bind) ? bind : &ts->pos_;
textslider *tb = widget_get_obj(c);
tb->pos = (bind) ? bind : &tb->pos_;

render_text(c);

return c;
}
11 changes: 11 additions & 0 deletions src/game/protos/scene.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,17 @@ int scene_create(scene *scene, game_state *gs, int scene_id) {

int scene_load_har(scene *scene, int player_id) {
game_player *player = game_state_get_player(scene->gs, player_id);
game_player *player0 = game_state_get_player(scene->gs, 0);
if(scene->af_data[player_id]) {
af_free(scene->af_data[player_id]);
omf_free(scene->af_data[player_id]);
}

if(player_id > 0 && player->pilot->har_id == player0->pilot->har_id) {
scene->af_data[player_id] = scene->af_data[0];
return 0;
}

scene->af_data[player_id] = omf_calloc(1, sizeof(af));

int resource_id = har_to_resource(player->pilot->har_id);
Expand Down Expand Up @@ -198,6 +205,10 @@ void scene_free(scene *scene) {
}
bk_free(scene->bk_data);
omf_free(scene->bk_data);
if(scene->af_data[0] && scene->af_data[0] == scene->af_data[1]) {
// players were using the same HAR
scene->af_data[1] = NULL;
}
if(scene->af_data[0]) {
af_free(scene->af_data[0]);
omf_free(scene->af_data[0]);
Expand Down
13 changes: 7 additions & 6 deletions src/utils/str.c
Original file line number Diff line number Diff line change
Expand Up @@ -286,13 +286,14 @@ bool str_to_long(const str *string, long *result) {
return (string->data != end);
}

bool str_to_int(const str *string, int *result) {
long value;
bool got = str_to_long(string, &value);
if(got) {
*result = clamp_long_to_int(value);
bool str_to_int(const str *string, size_t pos, int *result) {
char *end;
*result = strtol(string->data + pos, &end, 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please bounds-check pos (aka offset) param, at least with an assert()

if(string->data + pos != end) {
*result = clamp_long_to_int(*result);
return true;
}
return got;
return false;
}

const char *str_c(const str *string) {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/str.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ bool str_to_long(const str *string, long *m);
* @return true If success
* @return false If failure (result value is invalid)
*/
bool str_to_int(const str *string, int *m);
bool str_to_int(const str *string, size_t offset, int *m);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document the new offset param.


/**
* @brief Returns a C string compatible represantation of a string object.
Expand Down
Loading