-
Notifications
You must be signed in to change notification settings - Fork 16
Fix Dash Faction launcher's querying of MSAA levels in Direct3D 11 #352
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please stop mixing formatting and code style changes with actual behavior changes in one commit. This makes reviewing more time consuming. Splitting changes into multiple logically separate commits with messages describing what changed and optionally why, is the best way to mix such changes. You can leave it mixed in this PR, but please don't do it in the future.
game_patch/graphics/gr.cpp
Outdated
|
|
||
| // Fix `rf::gr::text_2d_mode`. | ||
| AsmWriter{0x0050BB40}.push(static_cast<uint8_t>(rf::gr::FOG_NOT_ALLOWED)); | ||
| constexpr uint8_t FOG_TYPE = static_cast<uint8_t>(rf::gr::FOG_NOT_ALLOWED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this constant really helps with anything and only makes the code longer, because it is obvious from the enum variant name what it is. You use it only in one place, one line later. I can understand if this was a "magic" number, but it is enum variant which logically is a constant too.
At least it should have more specific name. This is not the fog type, this is a fog type used for text. So maybe it should be named TEXT_FOG_TYPE. This way if there will be a need to change another fog type in this scope (e.g. for images or whatever), there won't be a name conflict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I felt uncomfortable with the fact that the the overload was dependent on the cast.
| std::vector<std::string> get_adapters() override; | ||
| std::set<Resolution> get_resolutions(unsigned adapter, unsigned format) override; | ||
| std::set<unsigned> get_multisample_types(unsigned adapter, unsigned format, bool windowed) override; | ||
| std::set<uint32_t> get_multisample_types(uint32_t adapter, uint32_t format, bool windowed) override; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all other functions use unsigned adapter so now code is inconsistent and messy. I suggest to cancel those type changes on method parameters
No description provided.