Skip to content

Fix Dash Faction launcher's querying of MSAA levels in Direct3D 11#352

Merged
rafalh merged 2 commits into
rafalh:masterfrom
is-this-c:df_launcher_msaa
Jan 6, 2026
Merged

Fix Dash Faction launcher's querying of MSAA levels in Direct3D 11#352
rafalh merged 2 commits into
rafalh:masterfrom
is-this-c:df_launcher_msaa

Conversation

@is-this-c

Copy link
Copy Markdown
Contributor

No description provided.

@rafalh rafalh left a comment

Copy link
Copy Markdown
Owner

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.

Comment thread 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);

Copy link
Copy Markdown
Owner

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.

Copy link
Copy Markdown
Contributor Author

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;

Copy link
Copy Markdown
Owner

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

@is-this-c is-this-c requested a review from rafalh December 30, 2025 03:45
@rafalh rafalh merged commit b2d61d9 into rafalh:master Jan 6, 2026
2 checks passed
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.

2 participants