Skript icon indicating copy to clipboard operation
Skript copied to clipboard

Add single-argument vector function overload (fixes #8265)

Open harshasiddartha opened this issue 1 month ago • 9 comments

Description

This PR implements the requested feature from issue #8265: adding a single-argument overload to the vector function where vector(n) creates a vector with all components equal to n, equivalent to vector(n, n, n).

Changes

  • Added single-argument vector function overload in DefaultFunctions.java
  • Added comprehensive tests in ExprVectorFromXYZ.sk to verify the functionality
  • Updated documentation with examples

Motivation

This is useful for developers who require all-n vectors more easily. A common use-case would be display scale of a display entity. This is also included in many other vector libraries as a common overload.

Testing

Added tests that verify:

  • Basic cases (0, positive, negative, decimal values)
  • Random value testing (60 iterations)
  • Component equality verification

All tests follow the existing test patterns in the codebase.

Checklist

  • [x] I have read the contributing guidelines
  • [x] Code follows the existing code conventions
  • [x] Tests have been added
  • [x] Documentation has been updated

Fixes #8265

harshasiddartha avatar Oct 31 '25 19:10 harshasiddartha

Make sure you're targeting dev/feature and not master

EquipableMC avatar Oct 31 '25 20:10 EquipableMC

Thanks for the feedback! I've updated the PR to target dev/feature instead of master as requested.

harshasiddartha avatar Oct 31 '25 20:10 harshasiddartha

Hi! I see the CI checks are failing. Could you please provide more details about what's causing the build failures? The checkstyle check passed, so the code style is correct.

I've verified:

  • The function registration follows the same pattern as other functions
  • The syntax is correct
  • Function overloading is supported in Skript (as seen in the test file function overloading.sk)

The implementation adds a single-argument vector overload that creates vector(n, n, n) when called with vector(n). If there's a specific issue with the tests or the implementation, please let me know and I'll fix it!

harshasiddartha avatar Oct 31 '25 21:10 harshasiddartha

Updated the version from "2.13" to "2.13-pre1" as requested. The change has been pushed to the branch.

harshasiddartha avatar Oct 31 '25 21:10 harshasiddartha

Updated the version from "2.13" to "2.13-pre1" as requested. The change has been pushed to the branch.

it should be INSERT VERSION

erenkarakal avatar Oct 31 '25 21:10 erenkarakal

Updated the version to "INSERT VERSION" as suggested. This will be filled in with the correct version during the release process.

harshasiddartha avatar Oct 31 '25 21:10 harshasiddartha

Fixed the build failure! The issue was that Namespace.addSignature() doesn't support function overloads (it uses only the function name as the key, not parameter types).

I've updated Functions.register() to check if a signature with the same name already exists before adding it to the namespace. This allows function overloads to work correctly - the actual function resolution happens in FunctionRegistry which properly supports overloads using parameter types.

The fix ensures that:

  • Multiple functions with the same name but different parameter counts/types can be registered
  • The FunctionRegistry (which handles actual function calls) correctly distinguishes between overloads
  • The Namespace (used for backwards compatibility) doesn't throw "function name already used" errors

Tests should now pass!

harshasiddartha avatar Nov 01 '25 08:11 harshasiddartha

We cannot update this PR without you allowing us to edit the branch. Please allow access for contributors.

sovdeeth avatar Nov 11 '25 01:11 sovdeeth

@harshasiddartha please allow access for contributors on your branch. We will close this if you do not, as we are unable to update merge it without access.

sovdeeth avatar Dec 02 '25 02:12 sovdeeth