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

Implement replay feature ( ͡° ͜ʖ ͡°) #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

matheusfaustino
Copy link

Hello guys o/

I think this project very cool and I'd like to contribute it somewhat. So, I thought in creating a replay system using the inputRedirection and I'd like to know what you think.

I can fix anything related with the code, organization, etc... if you wanna.

C is not my main language, but I'd like to improve it.

To test it, just type F4, save your input and then play it selecting the right name (it get from replay folder)

I hope you like it.

See you,
Bye

@matheusfaustino
Copy link
Author

I implemented it because I thought that the ideia of sharing the replay would be very nice.

I mean, imagine you are having a bad time in a level of a game and then, you can use the replay, from someone, to pass that level, that would be very cool. You can, too, reproduce a speedrun or anything else.

There are a lot of parts, in the code, that could be improved and I can improve it if you like the ideia of the replay.

I tested only on El Capitan, for now :/

Copy link
Owner

@Stary2001 Stary2001 left a comment

Choose a reason for hiding this comment

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

Good effort, thanks for contributing :)
I do like the idea of a replay function, but it may be better suited to on-3ds implementation.
Regardless, I've gone through the code and marked some things I find questionable. Please don't use asprintf - it will create a lot of small allocations and unless you're careful to free() all of them the program will leak memory. Fixed buffers (either local/stack or global) and snprintf is preferred.

#include <unistd.h>
#include <time.h>
#include <sys/time.h>

Copy link
Owner

Choose a reason for hiding this comment

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

You need to include stdint.h here - replay.c doesn't compile otherwise.

src/replay.c Outdated

if (REPLAY_PLAYING == 1) {
char *filename = NULL;
asprintf(&filename, "%s/%s%s", REPLAY_FOLDER, REPLAY_PLAY_NAME, REPLAY_FILENAME_SUFFIX);
Copy link
Owner

Choose a reason for hiding this comment

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

asprintf with no free - use a stack buffer with snprintf?

src/replay.c Outdated
char *filename = NULL;
uint32_t count = 0;
do {
asprintf(&filename, "%s/%s%d%s", REPLAY_FOLDER, REPLAY_FILENAME_PRE, count, REPLAY_FILENAME_SUFFIX);
Copy link
Owner

Choose a reason for hiding this comment

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

asprintf with no free - snprintf to stack buffer?

src/replay.c Outdated
}
else {
fclose(file_replay_writer);
asprintf(&REPLAY_MSG, "%s", "INFO: Replay saved.");
Copy link
Owner

Choose a reason for hiding this comment

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

asprintf - make replay_msg a single buffer that you don't change? ie. char replay_msg[512];

src/replay.c Outdated
asprintf(&filename, "%s/%s%s", REPLAY_FOLDER, REPLAY_PLAY_NAME, REPLAY_FILENAME_SUFFIX);
if (!(file_replay_reader = fopen(filename, "rb"))) {
toggle_replay_playing();
asprintf(&REPLAY_MSG, "%s", "Error: Replay file does not exists!");
Copy link
Owner

Choose a reason for hiding this comment

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

'exists' -> 'exist', asprintf to REPLAY_MSG

src/replay.c Outdated
}

// add new char
asprintf(&REPLAY_FILENAME_PRE, "%s%c\0", new_name, new_key);
Copy link
Owner

Choose a reason for hiding this comment

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

this is allocating a new buffer for every character. :(

src/replay.c Outdated
}

// add new char
asprintf(&REPLAY_PLAY_NAME, "%s%c\0", new_name, new_key);
Copy link
Owner

Choose a reason for hiding this comment

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

same as above - allocating a new buffer for each character.

src/replay.c Outdated
memcpy(new_name, REPLAY_PLAY_NAME, strlen(REPLAY_PLAY_NAME)+1);

if(strcmp(REPLAY_PLAY_NAME, "...") == 0) { // first time editing
strcpy(new_name, "");
Copy link
Owner

Choose a reason for hiding this comment

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

this really does new_name[0] = 0;

src/replay.h Outdated
#include <mach/clock.h>
#include <mach/mach.h>
#endif

Copy link
Owner

Choose a reason for hiding this comment

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

windows?

src/main.c Outdated
if (curr_state == ACCEPTING_INPUT && is_replay_playing() == 0 && is_replay_recording() == 1)
{
save_command_replay(
create_raw_cmd(hid_state, circle_state, cstick_state, touch_state, special_buttons)
Copy link
Owner

Choose a reason for hiding this comment

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

just pass the buffer that this function already created

@matheusfaustino
Copy link
Author

I already fixed all the things you said to me. I didn't know that asprintf had that kind of behavior, thanks ^^
I think that will be cool it on-3ds, I'll try something about it.

Thank you for your time, if you find anything else, just let me know :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants