mochadoom
mochadoom copied to clipboard
Fix some bugs mostly related to classic/vanilla Doom behavior
- Fix vanilla mouse event handling (in game, in menu)
- Fix initial value of novert (i.e. disable vertical movement by default)
- Fix fullscreen resize options (Nearest, Bilinear, Bicubic)
- Fix writing alternate config files i.e. when -config argument is used
- Fix/suppress some compile warnings, remove deprecated API usage
- Show message dialog on missing WAD files
- Improve logging
- Improve WAD handler cleanup
- Center application window
- Minor code cleanup and formatting
I'm looking at the changes this time, and unlike the previous MR, there are just formatting changes to files, like replacing tab indentation with spaces. GitHub displays 425 changes. This obfuscates your functional changes and it makes it hard to see which files are really affected. I don't like reformatting, but I'll accept code cleanup if it's done manually and not by a tool that does it automatically. Can you avoid formatting changes? It makes MRs hard to review. I prefer atomic commits:
Atomic commits:
- Only affect a single aspect of the system.
- Allow for greater understandability of changes
- Less effort to roll back changes when needed
- Easier bug identification
Your changelog is already well separated into different bullet point. Your commits should be the same. So one commit that does "Fix vanilla mouse event handling", one that's "Fix initial value of novert", another one that's "Fix fullscreen resize options", etc. One commit per bug fix, improvement or new feature. This would allow me to review much quicker.
Thanks for your comments. Will try to separate the PR to multiple commits soon.
(Please note that GitHub can ignore whitespaces during reviews)
Hey, @AXDOOMER I've split the changes into separate and meaningful commits.
Unfortunately, I can't avoid the formatting changes (401d4ed) at that point easily, however this is a separate commit. I think a consistent (and standard) code formatting can help understanding and maintaining the code (and even to avoid some bugs).
Sorry for not having merged this yet. I haven't forgotten about it and it will get merged within the next few months. I'm just too busy with full-time work + full-time studying.