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

Migrate Vector3 to vector #943

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

CadeusTheGreat
Copy link
Contributor

@CadeusTheGreat CadeusTheGreat commented Dec 13, 2024

Introduction

With the introduction of the Vector RFC seen in the release notes for 650

Changes

This PR migrates the documentation of Vector3 references to the new vector library where appropriate. Some references may need changing back due to the change required explained next.

More changes may be needed to make this request make sense to end users. Such as a dedicated page in the documentation for - the new library (Raised in PR #957 ), its application and methods - However this was out of scope for this PR.

This PR also compacts Vector3.new(0, 0, 0) to vector.zero where appropriate.

Please note that I understand that vector hasn't been properly announced yet and so this PR may need to sit for a little bit.

Checks

By submitting your pull request for review, you agree to the following:

  • This contribution was created in whole or in part by me, and I have the right to submit it under the terms of this repository's open source licenses.
  • I understand and agree that this contribution and a record of it are public, maintained indefinitely, and may be redistributed under the terms of this repository's open source licenses.
  • To the best of my knowledge, all proposed changes are accurate.

@CadeusTheGreat CadeusTheGreat requested review from a team as code owners December 13, 2024 17:42
@CadeusTheGreat CadeusTheGreat changed the title Migrate vector3 to vector Migrate Vector3 to vector Dec 13, 2024
@github-actions github-actions bot added education Changes the Education content engine guides Changes the Engine guides engine reference Changes the Engine API Reference documentation resources Changes the Resources content tutorials Changes the tutorials labels Dec 13, 2024
@Ramdoys
Copy link
Contributor

Ramdoys commented Dec 13, 2024

I do wonder if this would be merged or not (in its current state). Yes, the vector library is much more performant, it is also not complete yet from what I understand. This also creates an abstraction between Vector3 and Vector2, which may be confusing to new learners. I wonder if there would be a way to continue referencing Vector3, Vector2, in addition to vector that would avoid confusion. I say this due to the fact that Vector3 and Vector2 are still going to be data types (for obvious reasons), and I believe that Vector2 support will also becoming to the vector library. This actually could be pretty good, since it means Vector3 and Vector2 can act as data types for the two vectors, and then the vector library is used to deal with them. So maybe this whole confusion claim is kind of arbitrary, but I still thought it was good to mention. (Edit; This will probably not happen anytime soon, vector will be limited to Vector3.)

Nonetheless, I would like to point out that you wrote vector.**new** which would be invalid. Instead, use vector.create, the correct way to create a vector with the library. There's also situations that you haven't changed, like, Vector3:Dot, Vector3:Cross etc., though minor.

@CadeusTheGreat
Copy link
Contributor Author

My bad on the vector.new point, thanks for poiting that out. I'll commit a fix for that now. Was just quickly replacing Vector3 for vector haha! The extra functions of the vector library I was going to do in a later PR since I was aiming for the low hanging fruit but I can take a look at those too.

@CadeusTheGreat
Copy link
Contributor Author

PR #957 has been created by someone else to help with the migration. May update these pages to reference the new vector page instead of Vector3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
education Changes the Education content engine guides Changes the Engine guides engine reference Changes the Engine API Reference documentation resources Changes the Resources content tutorials Changes the tutorials
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants