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

Expose contact point and contact normal on VehicleWheel3D to scripting. #93900

Merged

Conversation

TestSubject06
Copy link
Contributor

@TestSubject06 TestSubject06 commented Jul 3, 2024

@TestSubject06 TestSubject06 requested review from a team as code owners July 3, 2024 15:54
@AThousandShips AThousandShips changed the title Draft Implementation to Proposal 10106: expose contact point and contact normal on VehicleWheel3D to scripting. Expose contact point and contact normal on VehicleWheel3D to scripting. (WIP) Jul 3, 2024
@TestSubject06
Copy link
Contributor Author

The style formatter threw up, and didn't really tell me why.

@AThousandShips
Copy link
Member

Indentation, you can set up a pre-commit hook to help, see here

@AThousandShips
Copy link
Member

Is this meant to be a daft? Then please convert it to a draft, see top left of the page

@TestSubject06
Copy link
Contributor Author

Indentation

Okay, but what file is it even complaining about the indentation for? Everything is exactly like all surrounding code, including the documentation.

@TestSubject06 TestSubject06 marked this pull request as draft July 3, 2024 16:01
@TestSubject06
Copy link
Contributor Author

Ah, I see. It didn't like that I had a space before the newline.

@TestSubject06
Copy link
Contributor Author

@AThousandShips What do I need to do to get this moving? Is there someone I should ping to get this on a list for a review?

@AThousandShips
Copy link
Member

We're in feature freeze until after we've released 4.3, until then focus is on bug fixes

@TestSubject06
Copy link
Contributor Author

Now that 4.3 is out, can I request a review of this?

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (rebased on top of master 568589c), it works as expected.

Testing project: truck_town_wheel_visualizer.zip

truck_town_wheel_visualizer.mp4

@TestSubject06
Copy link
Contributor Author

Is this good to go with normal github squash+merge, or do I need to manually squash all the commits first?

@AThousandShips
Copy link
Member

You need to squash the commits yourself, we do not use squash commits as GitHub currently do not allow combining squash and merge commits and we prefer having distinct merge commits as they make the history a lot easier to work with

@AThousandShips AThousandShips changed the title Expose contact point and contact normal on VehicleWheel3D to scripting. (WIP) Expose contact point and contact normal on VehicleWheel3D to scripting. Aug 26, 2024
Appease doctool by switching the order of the documentation page entries.

Update doc/classes/VehicleWheel3D.xml

Co-authored-by: Hugo Locurcio <hugo.locurcio@hugo.pro>

Update doc/classes/VehicleWheel3D.xml

Co-authored-by: Hugo Locurcio <hugo.locurcio@hugo.pro>

Update doc/classes/VehicleWheel3D.xml

Thanks, didn't see the typo in the suggested change. Brain auto corrected it.

Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
@TestSubject06 TestSubject06 force-pushed the 10106/raycast_info_on_wheels branch from 6de0e5e to b56a037 Compare August 28, 2024 14:29
@TestSubject06
Copy link
Contributor Author

Okay, everything's squashed up. Let me know if there's anything else I need to do here.

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Aug 29, 2024
@akien-mga akien-mga merged commit 9725c03 into godotengine:master Aug 29, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@TestSubject06 TestSubject06 deleted the 10106/raycast_info_on_wheels branch September 14, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose m_raycastInfo in VehicleWheel3D to scripting
4 participants