Skip to content

Fix demoserver max_wait skips happening even when they're not supposed to - #680

Merged
oldmud0 merged 5 commits into
masterfrom
fix-demo-max-wait
Mar 19, 2022
Merged

Fix demoserver max_wait skips happening even when they're not supposed to#680
oldmud0 merged 5 commits into
masterfrom
fix-demo-max-wait

Conversation

@Crystalwarrior

Copy link
Copy Markdown
Contributor

Fix demoserver max_wait logic being absolutely bonkers, causing random skips that make no sense, and actually comment this piece of code.
.demo reference used: https://cdn.discordapp.com/attachments/278576491191599104/949242991917301780/tech_support.demo

Also adds a /debug command. /debug 1 enables debug mode, /debug 0 disables it. What it does is show you the current duration before next playback() call, allowing you to visualize wait packets, tweak delays while editing demo, etc.

…m skips that make no sense

actually comment this piece of code
…e for each packet, allowing you to visualize wait packets.
@Crystalwarrior
Crystalwarrior requested a review from oldmud0 March 4, 2022 11:20
@Crystalwarrior Crystalwarrior added this to the 2.10 milestone Mar 4, 2022
@github-actions

github-actions Bot commented Mar 4, 2022

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Comment thread src/demoserver.cpp
{
bool ok;
int toggle = args.at(1).toInt(&ok);
if (ok && (toggle == 0 || toggle == 1)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok variable is redundant as toggle would be -1 on error and that is already checked by your if statement

@Crystalwarrior Crystalwarrior Mar 10, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

wouldn't .at(1) cause an error if args size is less than 1, rather than returning -1?

@oldmud0 oldmud0 Mar 19, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

args.size() > 1 holds true in this branch of code so it will not fail

also i'm clearly losing brain cells because toInt() returns 0 on failure, not -1. so actually it is not redundant

Comment thread src/demoserver.cpp Outdated
Comment thread src/demoserver.cpp Outdated
Crystalwarrior and others added 2 commits March 10, 2022 05:04
Co-authored-by: oldmud0 <oldmud0@users.noreply.github.com>
Co-authored-by: oldmud0 <oldmud0@users.noreply.github.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/demoserver.cpp Outdated
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@Crystalwarrior
Crystalwarrior requested a review from oldmud0 March 13, 2022 15:50
Comment thread src/demoserver.cpp
{
bool ok;
int toggle = args.at(1).toInt(&ok);
if (ok && (toggle == 0 || toggle == 1)) {

@oldmud0 oldmud0 Mar 19, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

args.size() > 1 holds true in this branch of code so it will not fail

also i'm clearly losing brain cells because toInt() returns 0 on failure, not -1. so actually it is not redundant

@oldmud0
oldmud0 merged commit aa2a29f into master Mar 19, 2022
@in1tiate
in1tiate deleted the fix-demo-max-wait branch July 18, 2022 22:49
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