Skip to content

This PR intended to fix #4724#4731

Open
dsFejerA wants to merge 4 commits intomultitheftauto:masterfrom
dsFejerA:AttachElements
Open

This PR intended to fix #4724#4731
dsFejerA wants to merge 4 commits intomultitheftauto:masterfrom
dsFejerA:AttachElements

Conversation

@dsFejerA
Copy link

@dsFejerA dsFejerA commented Feb 28, 2026

Summary

Fix the order of operations in CStaticFunctionDefinitions::AttachElements on the server-side. Move the ConvertDegreesToRadians(vecRotation) call to after SetAttachedOffsets, ensuring that rotation values are stored in degrees before being converted to radians for internal use. This restores the correct behavior of getElementAttachedOffsets, which should return rotation values in degrees to match the input to attachElements.

Motivation

#4726
#4724

Test plan

#4726
#4724

local obj1 = createObject(1365, 0, 0, 3);
local obj2 = createObject(1337, 0, 0, 3);

-- setTimer(attachElements, 50, 1, obj2, obj1, -2, 0, 2, 10, 20, 30);
attachElements(obj2, obj1, -2, 0, 2, 10, 20, 30);

iprint(getElementPosition(obj2));
iprint(getElementRotation(obj2));

setTimer(function()
    iprint(getElementAttachedOffsets(obj2));
end, 200, 1);

Checklist

  • Your code should follow the coding guidelines.
  • Smaller pull requests are easier to review. If your pull request is beefy, your pull request should be reviewable commit-by-commit.

@dsFejerA dsFejerA requested a review from a team as a code owner February 28, 2026 00:10
@FileEX
Copy link
Member

FileEX commented Feb 28, 2026

To fully fix the issue

spawning an 2 objects and immediately attaching them to one another used to work, now the rotation isn't being taken
into account. Workaround: setTimer with a 50ms interval to attach it 50ms later works.

the order in CElementRPCs::AttachElements also needs to be changed.

@FileEX FileEX added the bugfix Solution to a bug of any kind label Feb 28, 2026
@FileEX FileEX linked an issue Feb 28, 2026 that may be closed by this pull request
1 task
@dsFejerA
Copy link
Author

dsFejerA commented Feb 28, 2026

To fully fix the issue

spawning an 2 objects and immediately attaching them to one another used to work, now the rotation isn't being taken
into account. Workaround: setTimer with a 50ms interval to attach it 50ms later works.

the order in CElementRPCs::AttachElements also needs to be changed.

Additionally, the current situation looks like everything works perfectly on the client side.
(even If it’s created on the server side, then after synchronization it’s also fine.)

What I don’t understand, however, is why the logic was refined to “fix” one bug while another one was left there.

--server

DamonOne executed command:
getElementRotation(getElementsByType("object")[2])
Command results:
0 [number], 0 [number], 278.87329101562 [number]

DamonOne executed command:
getElementAttachedOffsets(getElementsByType("object")[2])
Command results:
-2 [number], 0 [number], 2 [number], 0 [number], 0 [number], 30 [number]
Executing client-side command: getElementRotation(getElementsByType("object")[2])
Command results: 0 [number], 0 [number], 30 [number]
Executing client-side command: getElementAttachedOffsets(getElementsByType("object")[2])
Command results: -2 [number], 0 [number], 2 [number], 0 [number], 0 [number], 30 [number]

That 278 (on server side) value is obtained from 30 radians, which equals 1718.8733854°.
We normalize it using %360, so:

1718.8733854 % 360 = 278.8733854°

I haven’t had time to fully look into things yet, but I think this should be fixed as well.

What do you think?

@FileEX
Copy link
Member

FileEX commented Mar 1, 2026

What I don’t understand, however, is why the logic was refined to “fix” one bug while another one was left there.

It’s normal when a PR does more than it should. The author of PR apparently overlooked it.

I haven’t had time to fully look into things yet, but I think this should be fixed as well.

Sure

@dsFejerA
Copy link
Author

dsFejerA commented Mar 2, 2026

To fully fix the issue

spawning an 2 objects and immediately attaching them to one another used to work, now the rotation isn't being taken
into account. Workaround: setTimer with a 50ms interval to attach it 50ms later works.

the order in CElementRPCs::AttachElements also needs to be changed.

Client doesn't need any adjustments, bugs mentioned in #4724 are fully fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Solution to a bug of any kind

Projects

None yet

Development

Successfully merging this pull request may close these issues.

attaching elements inconsistent and broken in 1.7

2 participants