Add vector2 lua API for 2D vectors#16929
Add vector2 lua API for 2D vectors#16929kromka-chleba wants to merge 43 commits intoluanti-org:masterfrom
Conversation
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
appgurueu
left a comment
There was a problem hiding this comment.
Dunno how to feel about AI slop, but for the sake of it, I took a quick look:
- Why is it inheriting all the deprecations from
vector? - The from/to 3d conversions are a bit nonsensical, imo: There is no practical reason why the extension should always be done with z = 0. I would prefer to avoid such conversions for now.
- If we are to have them, which axes x and y map to should be variable (and hence which axis is dropped).
- At which point we might as well omit the constructors and tell people to use
vector.new?
- I'm missing polar coordinates and signed angles
appgurueu
left a comment
There was a problem hiding this comment.
Oh also: push_v2* should probably be updated to push vectors with the proper metatable.
These models love boilerplate code. I will fix it.
This was actually my idea but then I considered it may be a bit stupid. So it's probably better to let the user decide?
I'll look into this. |
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
|
Also probably off-topic but when I asked about your AI policy I got this: Unless this was actually for ContentDB and not Luanti. |
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
|
Okay so I hopefully addressed the points about deprecated functionality and bad/misleading conversion functions. EDIT: |
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
|
Okay so we implemented these polar coordinates and signed angles. I see that the solution for signed angles is a bit verbose and perhaps instead we should use something more compact like this equation? https://wumbo.net/formulas/angle-between-two-vectors-2d/ |
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
|
There should be no problem with our AI policy in principle. The only thing this clearly "rips off" is the permissively licensed 3d vector implementation, so I don't think there is a substantial copyright concern here. Personally reviewing a larger AI generated PR is a bit of a new a thing for me, but so far the quality seems to be acceptable so I don't mind it too much. Perhaps the only substantial concern is one of PR (as in public relations, not pull request, hah): People who are avidly anti-AI might take an issue with us merging largely AI-generated PRs (even if they are of acceptable quality). 🤷 As for the signed angles, the core of the implementation (compute angle of both vectors via
The user ultimately needs to decide on which plane the coordinates lie in, yes. (Judging e.g. by the earlier "vector.flat" PR and similar, I feel like x-z would be pretty decent for many applications with mobs and such, but still there is no simple "general rule" and modders need to be explicit.) |
Co-authored-by: Lars Müller <34514239+appgurueu@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
|
The API mentions counterclockwise operations for example in |
Rotations in mathematics are typically counterclockwise. So rotating (1, 0) by 90° gives you (0, 1) for example. By these conventions, the formula for the point on the unit circle for a given angle is just x = cos(angle), y = sin(angle), which is what I would expect. So I think it's fine. |
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
|
Did you review the unit tests yet? The checkbox in your PR description is still unchecked, which has sparked some concerns in the dev IRC (https://irc.luanti.org/luanti-dev/2026-02-17#i_6314948). (I have reviewed them in full, but just to be sure you should too.) |
I did skim through them but did not do a proper review (was busy IRL). |
|
Do it whenever you have the time.
Hopefully not, but I also make mistakes. It is expected that you review code you contribute. It might also make the second reviewer's job a bit easier if they know there have already been two pairs of eyes on the code. |
|
Okay I did read all the tests, they look correct and I did check the angle operations, distance logic, etc. manually. Found one case where the test is correct but could be cleaner. |
…tors Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
|
I'll retest the feature in the game and reread the implementation once again to avoid more complaints towards my precious slop. Though the tests look reasonable and pass, which already means the implementation is fairly decent. |
|
Looks authentic and still works in the game. |
|
Will address the review later (not today). |
Co-authored-by: sfan5 <sfan5@live.de>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
|
sfan5 review hopefully addressed. |
…functions Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
|
I'm sorry but, @kromka-chleba you seriously had to ask Copilot to fix the documentation instead of doing that yourself? Like, the heck, it required not even 5 minutes, this is ridiculous. We did the homework for you, you only had to implement it. I'm honestly baffled and saddened at the same time; don't expect more reviews from me |
No but I'm lazy. The time to fetch the latest version from git, edit it without making mistakes, and push again would be the same or more.
What difference does it make? I spent 10 seconds on typing a prompt for this trivial change. Doing it manually doesn't make it any more meaningful or correct.
What homework, I know Lua and I know git. There's nothing to learn here.
If there's a problem with certain tools in Luanti development then make it your official policy rather than punishing people for implementing features. I have ADHD which makes it hard to focus on coding and I also have dyslexia which results in dumb typos and painful debugging. Copilot gets shit done and actually makes fewer mistakes than me. Probably also wastes less electricity than me wiggling on my chair for 2 weeks and recompiling buggy human-made code. Now I wasted 20 minutes typing this post instead of spending time with friends and/or working on my Master's. If using AI is a problem then just disallow AI contributions and I'll stop. EDIT: Also if the code devs who approved this PR think this was more work to review compared to a 100% human-made PR then say so. I personally believe I'd make similar mistakes or worse and it'd take more or equal time. But yeah, I don't want to give you more work than needed so if this was actually more work then I'll stop with AI PRs. |
That's my personal point of view, not Luanti team's as a whole
You had to get rid of "/ vector2.foo" in every line, how does ADHD and dyslexia impact that? Genuinely curious |
ADHD: I've been procrastinating most coding tasks for months and even years. This is because to focus on a task it needs to provide a decent dopamine feedback loop. Artistic tasks provide that, for example 3D modeling in Blender. Ordinary coding also does that, but less. That's why I've been primarily 3D modeling for my game for 2 years instead of coding. I've lately discovered that Github provides a free Copilot Pro for students and I tried it. The prompt-read-verify-correct loop provides dopamine boosts which keep me focused for hours (hyperfocus). Thanks to Copilot got back to the coding tasks I've been procrastinating (mostly luanti games) and also revisited my TODO list of features for Luanti I need for my personal projects. dyslexia: less frequent when I use english, which is not my native language. In coding that boils down to typos such as So thanks to Copilot I can actually focus on designing the interface and high-level features rather than typing buggy code manually. Reviewing AI code and testing it in the game is also more interesting and stimulating. EDIT: seems like I did not answer the question in the end due to focus defficiency lol. As for this exact task - prompting this is faster and less error prone. The first obstacle for this exact task would be getting motivation and focus, then I'd need to grep each piece and run the Emacs command for replacement. Misclicking something could result in skipping something or inserting an unneeded character which I could miss during reviewing my own code. I just accepted Copilot is better than me on simple tasks. It's english is also often way better. So faster, less error prone. |
|
Thanks for clarifying. I've never thought of these tools like helping tools for people with ADHD/dyslexia and I thought that, considering the initial "The whole PR is AI slop", it was only a huge displaying of laziness. I'm sorry for jumping the gun and for what I've said |
Goal of the PR
This pull request introduces a new lua API for 2D vectors (
vector2)Does it resolve any reported issue?
Part of this #16921
If you have used an LLM/AI to help with code or assets, you must disclose this.
The whole PR is AI slop.
To do
This PR is Ready for Review
How to test
Example code
Read the documentation and try it out I guess, there are also unit tests.
I could provide some better instructions later.