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

Add support for new DynamicCamMap checkbox. #1572

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

Hoikas
Copy link
Member

@Hoikas Hoikas commented Mar 2, 2024

This is the companion piece to H-uru/moul-assets#251. There is no longer any need to manually localize the options menu 🎉. I didn't bother with improving the quality of tangential Python code. All of the options menu Python is terrible, and I didn't want to get bogged down with that.

Copy link
Contributor

@dgelessus dgelessus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM at first glance (but haven't done any local testing yet)

@@ -827,6 +827,10 @@ def PtStartScreenCapture(selfKey,width=800,height=600):
"""Starts a capture of the screen"""
pass

def PtSupportsPlanarReflections() -> bool:
"""Returns if planar reflections are supported"""
...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with the rest of the file, and because this isn't a .pyi file:

Suggested change
...
pass

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the way that I've been doing all the rest of the new/updated API functions to ease the (eventual? hopeful?) transition to pyi. Example:

def PtYesNoDialog(cb: Union[None, ptKey, Callable], message: str, /, dialogType: int = PtConfirmationType.YesNo) -> None:
"""This will display a confirmation dialog to the user with the text `message`. This dialog
_has_ to be answered by the user, and their answer will be returned in a Notify message
or callback given by `cb`."""
...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, alright then - I guess I never noticed any of those yet. Yes, it would be nice to convert these to proper .pyi files at some point.

@@ -62,9 +62,10 @@
kGraphicsShadows = "Graphics.Shadow.Enable"
kGraphicsVerticalSync = "Graphics.EnableVSync"
kGraphicsShadowQuality = "Graphics.Shadow.VisibleDistance"
kGraphicsDynamicReflections = "Graphics.EnablePlanarReflections"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For anyone else wondering: this console command has already existed since forever.

This has been fixed in xKIGUI.max. If you don't have the updated PRPs,
the options menu will still work, it'll just be in English. Update your
doggone PRPs, yo.
Myst V didn't have this function either. That's interesting because, in
theory, we may be presented with a video card that doesn't support the
Direct3D caps needed to perform the effect. At this stage in the game,
everyone should support DCMs, so this is just an exercise in technical
correctness.
@Hoikas
Copy link
Member Author

Hoikas commented Mar 3, 2024

There seems to be a problem in the key map dialog where none of the key bindings are displaying. Investigating...

The call to show the key binding text was obscured by all of the manual
localization. Readd it.
@Hoikas
Copy link
Member Author

Hoikas commented Mar 3, 2024

Key bindings dialog is now working correctly.

@Hoikas Hoikas requested a review from Deledrius March 5, 2024 14:02
@Hoikas Hoikas merged commit 8cc47ca into H-uru:master Mar 7, 2024
18 checks passed
@Hoikas Hoikas deleted the options_menu_work branch March 7, 2024 00:41
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.

3 participants