search

wheybags / freeablo

[ARCHIVED] Modern reimplementation of the Diablo 1 game engine
GNU General Public License v3.0
2.16k stars 193 forks source link

added Misc::Point and in renderer.cpp Windows endings are replaced #375

Closed akuskis closed 6 years ago

akuskis commented 6 years ago

This PR should be bound to ticket #326 Misc::Point was added. I would like to add get/set functions and rx/ry (for getting references), but in general looks like variables in project are not private, so I've left them public too. Later it can be improved.

akuskis commented 6 years ago

In latest commit I've executed clang-format-6.0 on changed files. But anyway, only this check doesn't pass...

wheybags commented 6 years ago

Also, re: getters and setters, I normally only add a (const) getter, if I find myself adding a setter for it later I just make it public, because that';s what a getter/setter pair is anyway, just with nastier syntax. And sure, it's a bit of a pain later if you want to actually have the getters/setters run code, but that's what IDEs are for, and IMO it's better to have nicer code now than sacrifice readability for a minor timesaving that might well not even happen later.

On Sat, Jul 21, 2018 at 1:30 PM, Artem Kuskis notifications@github.com wrote:

In latest commit I've executed clang-format-6.0 on changed files. But anyway, only this check doesn't pass...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/wheybags/freeablo/pull/375#issuecomment-406789437, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCumn2wk54YlWHWqvaCMcFwIBVp2Zejks5uIxDxgaJpZM4VZjyo .

akuskis commented 6 years ago

Thank you for your comments! I've found before, that there was used special patch and clang-format v5. I see your point about using the same version and patching... just afraid that from time to time that patch should be updated. Yes, there are missed opportunities (as you said), because I'd like to get some feed back first. I'll take comments into account. Thank you for your review!

wheybags commented 6 years ago

Thanks!