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

Move player task-delay #3717

Merged
3 commits merged into from
Oct 23, 2021
Merged

Move player task-delay #3717

3 commits merged into from
Oct 23, 2021

Conversation

ramon-bernardo
Copy link
Contributor

@ramon-bernardo ramon-bernardo commented Oct 6, 2021

Pull Request Prelude

Changes Proposed

  • Move player task-delay

@ghost ghost requested review from DSpeichert, nekiro, ranisalt and yamaken93 October 6, 2021 18:11
@ghost
Copy link

ghost commented Oct 6, 2021

I think it would be nicer if we had them at game.h as we have these:

https://github.com/otland/forgottenserver/blob/master/src/game.h#L66-L69

@ramon-bernardo
Copy link
Contributor Author

ramon-bernardo commented Oct 6, 2021

I think it would be nicer if we had them at game.h as we have these:

https://github.com/otland/forgottenserver/blob/master/src/game.h#L66-L69

Its a good point. Changed.

@ramon-bernardo ramon-bernardo changed the title Move hardcoded player task-delay to config Move hardcoded player task-delay Oct 6, 2021
@ghost
Copy link

ghost commented Oct 6, 2021

that would still mean they are hardcoded, so no point in pr xD

@ramon-bernardo ramon-bernardo changed the title Move hardcoded player task-delay Move player task-delay Oct 6, 2021
@ranisalt
Copy link
Member

ranisalt commented Oct 6, 2021

that would still mean they are hardcoded, so no point in pr xD

At least we avoid the magic numbers sprinkled around

@ranisalt ranisalt closed this Oct 6, 2021
@ranisalt ranisalt reopened this Oct 6, 2021
@ranisalt
Copy link
Member

ranisalt commented Oct 6, 2021

Big finger on phone screen attacks again

@slawkens
Copy link
Contributor

slawkens commented Oct 9, 2021

Looks good, making code more readable.

However, I would use the same convention as other variables do, that is: first the prefix, then the rest of variable name. Like INTERVAL_*

Like this:

MOVE_CREATURE_INTERVAL ->INTERVAL_MOVE_CREATURE
RANGE_MOVE_CREATURE_INTERVAL -> INTERVAL_RANGE_MOVE_CREATURE

Other than that, LGTM!

@ramon-bernardo
Copy link
Contributor Author

Looks good, making code more readable.

However, I would use the same convention as other variables do, that is: first the prefix, then the rest of variable name. Like INTERVAL_*

Like this:

MOVE_CREATURE_INTERVAL ->INTERVAL_MOVE_CREATURE RANGE_MOVE_CREATURE_INTERVAL -> INTERVAL_RANGE_MOVE_CREATURE

Other than that, LGTM!

I didn't find use of interval on front, but I like interval_...

@slawkens
Copy link
Contributor

Ignore that, I was going too much into details 😉

@ghost ghost merged commit 5da493f into otland:master Oct 23, 2021
@Zbizu
Copy link
Contributor

Zbizu commented Oct 23, 2021

did it close #1845?

@ramon-bernardo
Copy link
Contributor Author

did it close #1845?

No, delay here is from the server and has nothing to do with packets.
Unfortunately we still don't have a solution for this.

This pull request was closed.
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.

4 participants