Skip to content

Conversation

eddynya
Copy link

@eddynya eddynya commented Nov 11, 2023

Death Counter

  • Keeps track of your deaths in story mode

Loadless Timers

  • Includes both fullgame and segment timers, and each can be toggled to not show at all, show at all times, show between worlds, and only show at the end of the run
  • If the segment timer is enabled in any way, at the end of the run, you can see all your split times + segment times for each world
  • Toggle-able warning that shows up on the name entry screen if no timers are enabled

Unified draw_timer() Code

  • The challenge mode segment timer, IW timer, RTA/Pause timer, and story timer all use the updated function

Known Issues

-timer no longer increments on mkb::STAGE_SELECTED due to (load?) variance
-timer no longer increments between the stage select a press and the start of the 10 ball screen spin in bc of variance between emu and console (will test more though to be sure)
-the timer now increments for exactly 49 frames during fade out (which is the usual fade out time when stage selecting) after pressing stage select. this fade out time should not vary between sd/usb/dolphin
-fixed the death counter bug where it would increment to 1 on the first stage automatically
Copy link
Owner

@ComplexPlane ComplexPlane left a comment

Choose a reason for hiding this comment

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

This should be plenty to get you started!

Copy link
Owner

Choose a reason for hiding this comment

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

You can undo these permissions changes with git checkout scripts inside WSL. I think this is caused by having the repo cloned outside WSL, or accessing the repo inside WSL with a tool outside WSL... I forget what.

Point is, IMO the best experience is to git clone the repo from a WSL terminal inside your WSL home directory, and open it in vscode using code . inside the folder in a WSL terminal. I don't know what your setup looks like but nambo/rehtrop should definitely be able to sort you out if there's an issue.

Copy link
Collaborator

@rehtropsmb rehtropsmb Nov 13, 2023

Choose a reason for hiding this comment

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

Setting up my environment on my laptop and I ran into this too. I believe when vscode isn't connected to WSL, it attempts to Windows-ify the files? I'm also not sure if code . sets up the remote connection if you don't already have the WSL extension installed.

@eddynya If you don't have the WSL extension for vscode, you'll want to get it before trying Complex's solution. Once you have it, do ctrl+shift+p and type "connect to wsl" and select that option. Then try running git checkout scripts and the files should be reverted.

If this doesn't work and you want a simpler solution you can try git config core.filemode false. This will stop git from picking up these file changes.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure about git config core.filemode false because we do want to respect executable bit of the script files and future scripts etc. You shouldn't run into this issue if you clone the repo inside your WSL home dir, and open code with code . (or Connect to WSL).

Comment on lines 43 to 47
// first framing should not increase the death counter, and retrying after breaking the tape should not increase it either
// to do: however, if you retry after breaking the tape on the very first frame (so the frame before goal init), it does count as a death when it should not
if (mkb::sub_mode == mkb::SMD_GAME_GOAL_INIT || mkb::g_storymode_stageselect_state == mkb::STAGE_SELECT_INTRO_SEQUENCE) {
s_can_die = false;
}
Copy link
Owner

Choose a reason for hiding this comment

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

Why should retrying on first frame not count as a death? Wouldn't it be simpler to keep it consistent with the game's behavior?

Comment on lines 116 to 117
if (mkb::g_storymode_stageselect_state == mkb::STAGE_SELECT_INTRO_SEQUENCE || mkb::g_storymode_stageselect_state == 3 ||
mkb::g_storymode_stageselect_state == mkb::STAGE_SELECT_IDLE || mkb::g_storymode_stageselect_state == 5 || mkb::g_storymode_stageselect_state == mkb::STAGE_SELECTED) {
Copy link
Owner

Choose a reason for hiding this comment

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

There's a StoryModeStageSelectState enum you can make use of, but it looks like it's missing 5

Copy link
Owner

Choose a reason for hiding this comment

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

Ok, at this point I actually think timerdisp is trying to be way too many things for its own good. I did suggest generalizing draw_timer(), but I didn't imagine you'd want to do stuff like optionally draw two timers on the same line in a specific format.

Here's the relevant concerns as I understand them:

  1. Multiple modules want to draw timers left-aligned at certain parts of the screen, but they should "stack" to avoid overlapping
  2. Modules need to format their timers differently (prefixes, more than one timer, etc.), but without duplicating excessive formatting code

Modules need to negotiate where their lines of text are rendered, but aside from that, they don't need to agree on how their timers are drawn.

So here's my proposal:

  1. Make a separate module that is dedicated to dishing out "slots" ((x, y) coordinate pairs) for other modules to render arbitrary lines of text to. Modules that call this function earlier get "slots" higher on the screen. The function might look like:
enum class Region {
    TopLeft,
    TopRight
};
Vec2d get_next_slot(Region pos);

Although, since your code is the only thing using the top left region, you might not need to implement this at all / just keep the current logic for how stuff in the top-right region is drawn.

  1. Let individual modules render their own arbitrary line of text. Modules could use a shared function to format a single frame counter variable as a time as a helper, and e.g. call it multiple times if they need to render more than one timer on the same line.

Btw, this is how you build a formatted string by itself, so you can use it later:

char str[32] = {}; // Must be big enough to hold the longest possible string, plus one null character
mkb::sprintf(buf, "int: %02d, float: %.02f, string: %s", 10, 1.5, "substring");

Using a shared timer formatting function would look something like:

char timer_str[32] = {};
timer_utils::format_timer(timer_str, my_framecount /* More options here */);
draw::debug_text("SEG: %s", timer_str);

Note that you can't returna pointer to the buffer you sprintf into because it is a stack variable, so you have to pass it "down" instead of "up".

Copy link
Collaborator

Choose a reason for hiding this comment

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

All the timer code here looks like it's gotten overly complex, so this sounds like a great idea! @eddynya I can help you get started with this, just dm me if you have questions!

Comment on lines 224 to 256
if (s_is_on_spin_in == true) {
// increment the timer every frame during spin in
s_spin_in_timer +=1;
}
if (s_is_on_gameplay == true) {
//increment the timer every frame during gameplay
s_gameplay_timer +=1;
}
if (s_is_postgoal == true && s_stage_fade_out_timer <= stage_fade_out_time) {
// increment the timer every frame after game goal init happens; once you press stage select, a separate 49 frame timer is started (fade out from stage select
// to the first completely white frame takes 49 frames). once the timer hits 49 frames, stop incrementing the timer
// until the 10 ball screen starts spinning in
s_postgoal_timer +=1;
}
if (mkb::g_storymode_stageselect_state == mkb::STAGE_SELECT_INTRO_SEQUENCE || mkb::g_storymode_stageselect_state == 3 ||
mkb::g_storymode_stageselect_state == mkb::STAGE_SELECT_IDLE) {
//increment the timer every frame on the story mode select screen until the a press input; we do not include the transition time after pressing a afterwards
// even ignoring completely white frames, the time spent on mkb::g_storymode_stageselect_state == mkb::STAGE_SELECTED can be
// highly variable (up to over 40 frames sometimes!), so for the purpose of a loadless timer, it makes sense to cut this out from the timer
s_stage_select_timer +=1;
}
if (s_is_on_exit_game_screen == true) {
// increment the timer every frame on the exit game screen
s_exit_game_timer +=1;
}
if (s_is_on_fallout_screen == true) {
// increment the timer every frame during the fallout sequence and y/n screen
s_fallout_timer += 1;
}
if (s_is_timeover == true) {
// increment the timer every frame during the timeover sequence
s_timeover_timer += 1;
}
Copy link
Owner

Choose a reason for hiding this comment

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

You probably want to use else for a lot of these...

@ComplexPlane
Copy link
Owner

Oh just noticed the conflicts... yeah you will need to pull and rebase/merge master to reconcile w/ rehtrop's latest changes

Comment on lines 33 to 46
static bool s_in_story;
static bool s_is_on_world[11];
static bool s_is_between_worlds;
static bool s_is_run_complete;
static bool s_is_on_spin_in;
static bool s_is_on_gameplay;
static bool s_is_postgoal;
static bool s_is_on_stage_select_screen;
static bool s_is_on_exit_game_screen;
static bool s_is_on_fallout_screen;
static bool s_is_timeover;
static bool s_can_increment_stage_counter;
static bool s_lower_stage_counter;
static bool s_start_STAGE_FADE_OUT_TIMEr;
Copy link
Owner

Choose a reason for hiding this comment

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

Vast majority of these don't need to be static variables (or perhaps not even variables at all)

Copy link
Author

Choose a reason for hiding this comment

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

I made most of these local variables, if you think most don't need to be variables at all I can do that. My reasoning for keeping them as variables is that (I hope) writing it this way makes it easier to read than having like 20 submodes in an if()

eddynya and others added 9 commits November 13, 2023 16:51
-rewrote completed stages code (will probably try and simplify still)
-small changes to death counter
Resolving merge conflicts with rehtrop's changes

-Changed the RTA/Pause timer to use the new function I made as a bandaid fix until I address Complex's comment
-Changed one of the timer widgets to say "Segment & Loadless Timers" until I decide how I want to reorganize this part of the menu
removed some static u32 variables that were being kept track of
Copy link
Collaborator

@rehtropsmb rehtropsmb left a comment

Choose a reason for hiding this comment

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

Some more stuff to take a look at. I still haven't tested in-game, this was just from doing an overview of the code. I'll do some gameplay testing eventually :)

Also make sure to remove the script changes from your PR! And pull in the most up-to-date code so you can fix your merge conflicts!

if (s_can_die == true && (mkb::sub_mode == mkb::SMD_GAME_READY_INIT || mkb::sub_mode == mkb::SMD_GAME_RINGOUT_INIT
|| mkb::sub_mode == mkb::SMD_GAME_TIMEOVER_INIT || mkb::sub_mode == mkb:: SMD_GAME_SCENARIO_RETURN)){
// you can die either by retrying after dropping in, falling out, timing over, or stage selecting after dropping in (but before breaking the tape)
s_death_count += 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

s_can_die would be set to false so it seems like this would only be able to be ran once anyways

Comment on lines 319 to 355
// if the segment timer is enabled in any capacity, show all 10 split times + iw times after the
// tape is broken on the last stage
if (TimerOptions(pref::get(pref::U8Pref::SegmentTimerOptions)) != TimerOptions::DontShow &&
is_run_complete) {
// haven't attempted the mkb::sprintf suggestion yet
timerdisp::draw_timer(IW_TIME_LOCATION_X, segment_timer_location_y, IW_TIME_TEXT_OFFSET,
"W1:", s_timer_group[0].split, s_timer_group[0].segment, true, false,
draw::WHITE);
timerdisp::draw_timer(IW_TIME_LOCATION_X, segment_timer_location_y + 1, IW_TIME_TEXT_OFFSET,
"W2:", s_timer_group[1].split, s_timer_group[1].segment, true, false,
draw::WHITE);
timerdisp::draw_timer(IW_TIME_LOCATION_X, segment_timer_location_y + 2, IW_TIME_TEXT_OFFSET,
"W3:", s_timer_group[2].split, s_timer_group[2].segment, true, false,
draw::WHITE);
timerdisp::draw_timer(IW_TIME_LOCATION_X, segment_timer_location_y + 3, IW_TIME_TEXT_OFFSET,
"W4:", s_timer_group[3].split, s_timer_group[3].segment, true, false,
draw::WHITE);
timerdisp::draw_timer(IW_TIME_LOCATION_X, segment_timer_location_y + 4, IW_TIME_TEXT_OFFSET,
"W5:", s_timer_group[4].split, s_timer_group[4].segment, true, false,
draw::WHITE);
timerdisp::draw_timer(IW_TIME_LOCATION_X, segment_timer_location_y + 5, IW_TIME_TEXT_OFFSET,
"W6:", s_timer_group[5].split, s_timer_group[5].segment, true, false,
draw::WHITE);
timerdisp::draw_timer(IW_TIME_LOCATION_X, segment_timer_location_y + 6, IW_TIME_TEXT_OFFSET,
"W7:", s_timer_group[6].split, s_timer_group[6].segment, true, false,
draw::WHITE);
timerdisp::draw_timer(IW_TIME_LOCATION_X, segment_timer_location_y + 7, IW_TIME_TEXT_OFFSET,
"W8:", s_timer_group[7].split, s_timer_group[7].segment, true, false,
draw::WHITE);
timerdisp::draw_timer(IW_TIME_LOCATION_X, segment_timer_location_y + 8, IW_TIME_TEXT_OFFSET,
"W9:", s_timer_group[8].split, s_timer_group[8].segment, true, false,
draw::WHITE);
// use segment timer spacing for w10 since "W10" is 3 characters long, not 2
timerdisp::draw_timer(SEGMENT_TIMER_LOCATION_X, segment_timer_location_y + 9,
SEGMENT_TIMER_TEXT_OFFSET, "W10:", s_timer_group[9].split,
s_timer_group[9].segment, true, false, draw::WHITE);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be it's own bool preference? Or just the EndOfRun case?

Copy link
Author

Choose a reason for hiding this comment

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

The way it works now is that if the segment timer is on at all (so either showing at all times, between worlds, or at the end of the run), when the run is finished, all 10 split and segment times are shown. I don't think this should get its own bool pref since this information is shown after the run, and so people who want to run pace blind aren't affected by it

eddynya and others added 24 commits November 21, 2023 17:42
retrying on the tape break frame no longer increases the death counter
nothing significant really
-timer and seg timer are consistently offset from each other currently when they shouldn't be
also fixed the update order bug causing the segment timer to be 1f behind what it should be
issue: the w1 split does not show up
the segment timer now stops on the goal tape break frame instead of using SMD_GOAL_INIT and subtracting 2 frames manually
-removed unused variables
-fixed error where split 1 was off by 1f
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.

3 participants