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

Explain TOGGLE_PAUSED_RENDERPHASES #912

Merged
merged 3 commits into from
Apr 21, 2024
Merged

Conversation

DatGuy1
Copy link
Contributor

@DatGuy1 DatGuy1 commented Aug 27, 2023

Also rename variable

Copy link
Contributor

@Z3rio Z3rio left a comment

Choose a reason for hiding this comment

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

Is the change from toggle to rendering really needed?
In my opinion, toggle does sound better.

Other than that, the descriptions looked fairly good to me.
the param description could probably be a bit better/easier to understand though, such as: Whether to free the camera or not

@4mmonium
Copy link
Contributor

I think the original parameter name is fine, not sure why it would need a name change.
The original description needs improvements though. This function pauses game rendering when toggled off and resumes when toggled on (very similar to what the game does in single player when you press ESC to open the pause menu when the world is loaded).

@DatGuy1
Copy link
Contributor Author

DatGuy1 commented Dec 17, 2023

I renamed it from 'toggle' to rendering because the native name together with the argument name doesn't make it clear which status it toggles to, which is the primary function of an argument name. Does it toggle rendering on? Does it toggle paused rendering on, meaning it doesn't render?

@Z3rio
Copy link
Contributor

Z3rio commented Dec 17, 2023

I renamed it from 'toggle' to rendering because the native name together with the argument name doesn't make it clear which status it toggles to, which is the primary function of an argument name. Does it toggle rendering on? Does it toggle paused rendering on, meaning it doesn't render?

The variable/param name "toggle" sounds much better though, and is uniform with the rest of the documentation.
As for the issue you are explaining with that name, that can be solved by simply adding descriptions for the native & param, which you seem to be doing in this PR.

```

Freezes the screen in its current state.
Copy link
Contributor

Choose a reason for hiding this comment

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

Better description would be Switches the rendering display to exclude everything except PostFX, resulting in a frozen screen before the UI pass.


## Parameters
* **toggle**:

* **rendering**: Whether to actively render the camera
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be toggle since it's a "toggle" native.

Change back to `toggle`, also clarify what the parameter does.
Thanks to @PsychoShock for the suggested description.
Copy link
Contributor

@4mmonium 4mmonium left a comment

Choose a reason for hiding this comment

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

LGTM, merging, thank you! 😊

@4mmonium 4mmonium merged commit ac7788f into citizenfx:master Apr 21, 2024
1 check 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.

None yet

4 participants