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

Cleanup: More documentation fixes #8208

Merged
merged 2 commits into from Jun 9, 2020
Merged

Conversation

@techgeeknz
Copy link
Contributor

techgeeknz commented Jun 8, 2020

This is in preparation for some major refactoring of the dirty block system (not to be confused with the dirty rectangle system) that is used by gfx.cpp to track portions of the screen that have been invalidated but not yet redrawn (at least, that is how I think it works; still investigating this).

That, in itself, is in preparation for moving the responsibility of drawing the mouse pointer (and, probably, the "network chat message"; whatever that is) into the video driver, so it can be updated independently of the rest of the screen (moving toward fixing #7006).

Copy link
Contributor

Yexo left a comment

Happy to approve the first commit, but not the 2nd one as it stands.

@@ -3478,6 +3478,8 @@ void ReInitAllWindows()
* @param clss The class of the window to position.
* @param setting The actual setting used for the window's position.
* @return X coordinate of left edge of the repositioned window.
*
* @todo Replace \c setting magic numbers with an enumeration?

This comment has been minimized.

Copy link
@Yexo

Yexo Jun 8, 2020

Contributor

I don't like todo's like this. I agree that the function would be cleaner without the magic numbers, but it's relatively easy to make lots of todo's like this that don't add any value but distract from the actual code. For small cleanups like this I'd rather leave out the todo than add it.

This comment has been minimized.

Copy link
@techgeeknz

techgeeknz Jun 8, 2020

Author Contributor

Point taken. The code is probably already littered with too many "TODO" and "XXX" comments, so I'll try to keep them to myself in future. At the very least, we can take solace in the fact that one of the previously documented todos has actually been done :)

@techgeeknz techgeeknz force-pushed the techgeeknz:master_docfix branch from f6340e6 to dc22eb4 Jun 8, 2020
@Yexo
Yexo approved these changes Jun 9, 2020
Copy link
Contributor

Yexo left a comment

Thanks for your contribution :)

@LordAro LordAro merged commit ee570e1 into OpenTTD:master Jun 9, 2020
8 checks passed
8 checks passed
Commit checker
Details
OpenTTD CI Build #20200608.8 succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.9) Linux linux-amd64-clang-3.9 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
@techgeeknz
Copy link
Contributor Author

techgeeknz commented Jun 9, 2020

Thanks for your contribution :)

You're more than welcome, master.

@LordAro
Copy link
Member

LordAro commented Jun 10, 2020

You should stop with the "master" stuff. It's weird...

@techgeeknz
Copy link
Contributor Author

techgeeknz commented Jun 13, 2020

You should stop with the "master" stuff. It's weird...

Sorry, I thought that's how we did things around here. On my first pull request, someone implied I should thank the person who merged it in.

@LordAro
Copy link
Member

LordAro commented Jun 13, 2020

"master" is the name of the branch it's being merged into, not the name of the person who did it...

@techgeeknz
Copy link
Contributor Author

techgeeknz commented Jun 13, 2020

"master" is the name of the branch it's being merged into, not the name of the person who did it...

I thought it could also be a noun by proxy for the person who is merging to master. Regardless, I'll stop with the "weirdness". 😉

@andythenorth
Copy link
Contributor

andythenorth commented Jun 13, 2020

Ha ha, that 'say thanks master' was in some very random looking comments from kawayanslayer.

No idea who / what kawayanslayer is, looked like someone testing an email bot to me, or it was just spam. 😄

Anyway, you can ignore that 😉

@techgeeknz
Copy link
Contributor Author

techgeeknz commented Jun 13, 2020

Thanks, master. Okay, I'll stop now 😎.

@techgeeknz
Copy link
Contributor Author

techgeeknz commented Jun 13, 2020

It also seems rather coincidental that @kawayanslayer joined GitHub on that very same day and posted an initial one-line commit to a repository they created 🤔.

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

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.