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

Node: XRToolsPickupModeChanger #280

Merged
merged 9 commits into from
Nov 21, 2022
Merged

Node: XRToolsPickupModeChanger #280

merged 9 commits into from
Nov 21, 2022

Conversation

pietru2004
Copy link
Contributor

Changes pickable original mode to selected one.

Usefull when collectable is set to static so for example randomly throwed item or move of npc won't make it move

@BastiaanOlij
Copy link
Member

Hmmm, not sure if I fully follow what is going on. Am I correct in assuming a use case where an object starts of as static, remains static when the user picks it up, but then you want it to become a rigid body after the user throws it?

Seems a bit of a roundabout way to implement that. It's not very clear from the object that this is its usage.
I think it would be better to implement this in pickable.gd itself and make an export variable like:

@export (int, "Default", "Rigid", "Static") var release_mode = 0

And then in the pick_up function change:

	# Remember the mode before pickup
	original_mode = mode if release_mode ==0 else release_mode - 1

@pietru2004
Copy link
Contributor Author

well yes it is my workaround for making object rigid after drop, but it also is in reverse case - if you pickup item, but after drop you want it static

Basicaly I have some Grapes on bush in game and they are set static so only way to make them rigid is by player pickup.

@BastiaanOlij
Copy link
Member

Happy for this to go in if @Malcolmnixon also agrees. Please do squash your commits so we just get the end result in one commit :)

@Malcolmnixon
Copy link
Collaborator

I have a proposal which I believe makes the code slightly easier to understand, and prevents enum-math.

Firstly define a ReleaseMode enum (please put after the last enum to keep them together and linting tools happy):

## Mode to set the body to when released
enum ReleaseMode {
	DEFAULT = -1,		## Preserve original mode when picked up
	RIGID = 0,			## Release and make rigid (MODE_RIGID)
	STATIC = 1			## Release and make static (MODE_STATIC)
}

Putting the "Default" as -1 allows the RIGID and STATIC options to have exactly the same values as the RigidBody Mode enumeration. Alas you have to use the numeric values rather than the enum values as gdscript does not consider them constants.

Secondarily define the export as follows:

## Release mode to use when releasing the object
export (ReleaseMode) var release_mode : int = ReleaseMode.DEFAULT

This exposes the enum options without having to provide string literals and clarifies what the initialized value is.

Finally the pickup function change can now be:

	# Remember the mode before pickup
	original_mode = mode if release_mode == ReleaseMode.DEFAULT else release_mode

I prefer this as it removes the magic '0' test and the '-1' math.

@BastiaanOlij
Copy link
Member

Indeed, Malcolms version is slightly neater then my suggestion.

@Malcolmnixon
Copy link
Collaborator

Let's also rename DEFAULT to ORIGINAL as it may be clearer as to what's going on.

@pietru2004
Copy link
Contributor Author

Gona apply it when I have minute

@pietru2004
Copy link
Contributor Author

erm just ignore
obraz
I was syncing fork

Copy link
Collaborator

@Malcolmnixon Malcolmnixon left a comment

Choose a reason for hiding this comment

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

Looks good and works fine when tested with a few hammers in all configurations.

@Malcolmnixon Malcolmnixon merged commit 22a9cf6 into GodotVR:master Nov 21, 2022
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