Uniformize layer names, script methods and documentation #51532
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pouleyKetchoupp
force-pushed
the
layer-mask-accessors
branch
from
23ad4e8
to
a9a9027
Compare
kleonc
reviewed
Aug 12, 2021
doc/classes/Camera3D.xml
Outdated
| <description> | ||
| Enables or disables the given [code]layer[/code] in the [member cull_mask]. | ||
| Sets the specified [code]value[/code] of the [member cull_mask] to [code]true[/code] or [code]false[/code], given a [code]layer_number[/code] between 1 and 20. |
There was a problem hiding this comment.
This description doesn't make much sense because [code]value[/code] refers to the passed bool, not to the value in cull_mask associated with the given layer. So I'd change it to something like:
Suggested change
| Sets the specified [code]value[/code] of the [member cull_mask] to [code]true[/code] or [code]false[/code], given a [code]layer_number[/code] between 1 and 20. | |
| Sets the specified value in the [member cull_mask] to [code]value[/code], given a [code]layer_number[/code] between 1 and 20. |
or:
Suggested change
| Sets the specified [code]value[/code] of the [member cull_mask] to [code]true[/code] or [code]false[/code], given a [code]layer_number[/code] between 1 and 20. | |
| Sets the value for the specified layer in [member cull_mask] to [code]value[/code], given a [code]layer_number[/code] between 1 and 20. |
or:
Suggested change
| Sets the specified [code]value[/code] of the [member cull_mask] to [code]true[/code] or [code]false[/code], given a [code]layer_number[/code] between 1 and 20. | |
| Based on [code]value[/code], enables or disables the specified layer in the [member cull_mask], given a [code]layer_number[/code] between 1 and 20. |
The same issue is present in other similar changes (I'm not commenting them one by one though).
There was a problem hiding this comment.
Makes sense! I've updated the documentation based on your third suggestion, and also updated the get_ functions to use similar wording.
- Back to 1-based layer names to make it clearer in editor UI - Layer bit accessors are renamed to layer value and 1-based too - Uniform errors and documentation in render and physics - Fix a few remaining collision_layer used in place of collision_mask
pouleyKetchoupp
force-pushed
the
layer-mask-accessors
branch
from
a9a9027
to
989acbb
Compare
akien-mga
approved these changes
Aug 12, 2021
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change based on discussion started in #44809 (comment), in order to solve godotengine/godot-proposals#2050 and still keep the editor layer mask UI intuitive.
set/get_layer_mask_bit(bit)functions are renamed toset/get_layer_mask_value(layer_number)and they use 1-based indices toocollision_layerused in place ofcollision_maskin physics