Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

added resource: Chemical Ladder v2.0 (ladders)#547

Open
ChemicalCreations wants to merge 1 commit intomultitheftauto:master from
ChemicalCreations:master
Open

added resource: Chemical Ladder v2.0 (ladders) #547
ChemicalCreations wants to merge 1 commit intomultitheftauto:master from
ChemicalCreations:master

Conversation

@ChemicalCreations
Copy link

@ChemicalCreations ChemicalCreations commented Sep 3, 2024 •
edited
Loading

Adds a resource that allows you to climb various ladders across SA map. Default ladders can be added or disabled via exported functions. This also allows you to add ladders to elements based on modelID (vehicle and object supported).
https://youtu.be/K3JhbzGyQto?si=84lkPPM4I3R189mh

Fernando-A-Rocha, NetroX1993, and Wannacry-ops reacted with heart emoji bum8hj and Wannacry-ops reacted with rocket emoji
Copy link
Contributor

jlillis commented Sep 4, 2024

Looks pretty cool. This is going to take a while to code review, but the first step would be to look at what the linter has flagged and address those issues.

Just to confirm on the record - are you the original author of the resource?

Copy link
Author

For the record yes.

Copy link
Contributor

Awesome scripts! Looking forward to this

Copy link
Member

Any updates on this?

Copy link
Member

Any updates on this?

[...] the first step would be to look at what the linter has flagged and address those issues.

This step was not done as far as I can see, so no, there are no updates.
Nico8345 reacted with thumbs up emoji

local climbers = {}
local climbs = {}

local anim = anims
Copy link
Member

Choose a reason for hiding this comment

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

This is unnecessary, because the anims table is a global variable, so it is accessible in this script from the global namespace

Suggested change
local anim = anims

Comment on lines +79 to +100
local minus
do
local abs = math.abs
-- calculate and return the distance unit2
-- needs to move to reach unit1
function minus(unit1, unit2)
local phi = abs(unit2-unit1) % 360
local sign = 1
-- used to calculate sign
if not ((unit1-unit2 >= 0 and unit1-unit2 <= 180)
or (unit1-unit2 <= -180 and unit1-unit2 >= -360)) then
sign = -1
end
if phi > 180 then
result = 360-phi
else
result = phi
end

return result*sign
end
end
Copy link
Member

Choose a reason for hiding this comment

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

This can be implemented much more simply, how about this solution?

Suggested change
local minus
do
local abs = math.abs
-- calculate and return the distance unit2
-- needs to move to reach unit1
function minus(unit1, unit2)
local phi = abs(unit2-unit1) % 360
local sign = 1
-- used to calculate sign
if not ((unit1-unit2 >= 0 and unit1-unit2 <= 180)
or (unit1-unit2 <= -180 and unit1-unit2 >= -360)) then
sign = -1
end
if phi > 180 then
result = 360-phi
else
result = phi
end
return result*sign
end
end
function minus(unit1, unit2)
local diff = unit2 - unit1
local diffNormalized = diff % 360
return diffNormalized > 180 and diffNormalized - 360 or diffNormalized
end

Copy link
Author

@ChemicalCreations ChemicalCreations Jan 16, 2025

Choose a reason for hiding this comment

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

Looks good, don't see why not as long as it works.

function getPedsOnLadder(surface)
local peds = {}
for ped, data in pairs(climbers) do
if surface==nil then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if surface==nil then
if notsurface then

Copy link
Author

@ChemicalCreations ChemicalCreations Jan 16, 2025

Choose a reason for hiding this comment

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

I wouldn't recommend that. nil and false aren't the same, they are done like that for a reason.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't recommend that. nil and false aren't the same, they are done like that for a reason.

In this case, it will not change how it works, and the change I suggested is a better solution

function isPedLadderClimbingEnabled(ped)
local eType = ped and isElement(ped) and getElementType(ped)
assert(eType=="ped" or eType=="player") -- isPed
if climbers[ped]==false then return false end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if climbers[ped]==false then return false end
if notclimbers[ped] then return false end

local dist = ((tx-sx)^2+(ty-sy)^2+(tz-sz)^2)^.5+10
if ((tx-px)^2+(ty-py)^2+(tz-pz)^2)^.5<dist or ((px-sx)^2+(py-sy)^2+(pz-sz)^2)^.5<dist then
local dist, p = getPointDistanceFromLadder(px, py, pz, sx, sy, sz, tx, ty, tz)
if (climbD==nil or dist<climbD) then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (climbD==nil or dist<climbD) then
if (notclimbD or dist<climbD) then

local eType = ped and isElement(ped) and getElementType(ped)
assert(eType=="ped" or eType=="player") -- isPed
if climbers[ped]==false then return false end
return climbers[ped]==nil or (climbers[ped] and true)
Copy link
Author

@ChemicalCreations ChemicalCreations Jan 16, 2025

Choose a reason for hiding this comment

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

Do realize this is supposed to return true when it's nil.

return false
end

do
Copy link
Member

Choose a reason for hiding this comment

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

Why is this do block necessary?

function removeLadder(surface, ladder)
local surfaceData = climbs[surface]
assert(surfaceData)
if ladder==nil then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ladder==nil then
if notladder then

Comment on lines +249 to +253
addDebugHook("preFunction", function(sourceResource, functionName, isAllowedByACL, luaFilename, luaLineNumber, ped) -- just to deal with freeroam
if sourceResource~=resource and climbers[ped] then
return "skip"
end
end, {"setPedAnimation"})
Copy link
Member

Choose a reason for hiding this comment

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

Not a good idea

Copy link
Author

@ChemicalCreations ChemicalCreations Jan 16, 2025

Choose a reason for hiding this comment

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

Agreed

Comment on lines +346 to +347
addDebugHook("postFunction", function(sourceResource, functionName, isAllowedByACL, luaFilename, luaLineNumber, ...)
end, {"createVehicle", "Vehicle", "createObject", "Object"})
Copy link
Member

Choose a reason for hiding this comment

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

Not used for anything

local eType = ped and isElement(ped) and getElementType(ped)
assert(eType=="ped" or eType=="player") -- isPed
if climbers[ped]==false then return false end
return climbers[ped]==nil or (climbers[ped] and true)
Copy link
Author

@ChemicalCreations ChemicalCreations Jan 16, 2025

Choose a reason for hiding this comment

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

Do realize this is supposed to return true when it's nil.

Copy link
Contributor

Xenius97 commented Feb 8, 2025

This resource is very unoptimized.
5 debug hooks?? Are you serious? RIP server performance

Copy link
Author

This resource is very unoptimized.
5 debug hooks?? Are you serious? RIP server performance

Is debughook really that heavy? Even with specifying the function/event???

Copy link
Contributor

Xenius97 commented Feb 8, 2025 •
edited
Loading

Is debughook really that heavy? Even with specifying the function/event???

Yes, we used just one in the past for logging a single function, and the server performance dropped by about 60%. And you have 5 hooks.

Copy link
Author

And you got the same result with this resource??? Or is it something you haven't tried.

Copy link
Contributor

Xenius97 commented Feb 8, 2025

No i've didnt use this resource, just noticed this. But this can cause problem on bigger servers.
Nico8345 reacted with thumbs up emoji

Copy link

ghost commented Feb 8, 2025

And you got the same result with this resource??? Or is it something you haven't tried.

The debug part is there for a reason.
obraz

Nico8345 reacted with thumbs up emoji

Copy link
Contributor

PlatinMTA commented Feb 8, 2025 •
edited
Loading

This resource is very unoptimized.
5 debug hooks?? Are you serious? RIP server performance

Is debughook really that heavy? Even with specifying the function/event???

Its heavy depending on how many times you use those functions but yeah, it's heavy. You really should change your hooks to events and timers that check new objects. How many times would you actually create an object thats a ladder? Not that many, so either you check them every minute or so with a timer, you check them after other resource starts or you expect the server owner to communicate to ladders that a new ladder has been added.

Rule of thumb, if you are using debugHook for something other than anticheat you are probably doing something wrong. Even then its not recommended. For example onElementDestroy shouldnt be on a debugHook, just add an event and check there if the object of a ladder has been destroyed.

Copy link
Author

Rule of thumb, if you are using debugHook for something other than anticheat you are probably doing something wrong. Even then its not recommended. For example onElementDestroy shouldnt be on a debugHook, just add an event and check there if the object of a ladder has been destroyed.

That's great but MTA doesn't offer element creation event Sooooo....... what's the next best thing yk. like you said, on resource start check, and probably checking elements type index if it changes etc, would be a better implementation

Copy link

ghost commented Feb 8, 2025 •
edited by ghost
Loading

Rule of thumb, if you are using debugHook for something other than anticheat you are probably doing something wrong. Even then its not recommended. For example onElementDestroy shouldnt be on a debugHook, just add an event and check there if the object of a ladder has been destroyed.

That's great but MTA doesn't offer element creation event Sooooo....... what's the next best thing yk. like you said, on resource start check, and probably checking elements type index if it changes etc, would be a better implementation

You don't need element creation event, simply use stream-in/out event, this is when an element gets rendered (created) for client.

https://wiki.multitheftauto.com/wiki/OnClientElementStreamIn

@ghost ghost force-pushed the master branch from 76eb9c6 to 31a4311 Compare September 15, 2025 08:37
Copy link

ghost commented Sep 15, 2025

@ChemicalCreations are you planning to work further on it?

Copy link
Contributor

Although it looks good in the video I don't think it should be added to the official resources for the same reason all the other amazing scripts that are on community.mtasa.com are not official resources because then there would be too many. ccw made a load of shaders like detail_shader that makes the game look better but didn't add them to official resources and instead they are on community.mtasa.com
omar-o22 reacted with thumbs up emoji

Copy link
Author

@ChemicalCreations are you planning to work further on it?

Yes, I do. My plan is to add the ability to jump upwards ladders (faster climbing), more checks for getting smacked off a ladder (say if you've got a ladder on a moving platform and lastly go over the concerns for the linter to begin with.

For the debug hook case, I get it can be slow, but I I've tested with /ipb and had no performance issues, but I don't own a big server so someone would have to tell me. Is it really the case that looping over all the elements on a timer has a lower performance impact that a onetime check?

Copy link
Author

Although it looks good in the video I don't think it should be added to the official resources for the same reason all the other amazing scripts that are on community.mtasa.com are not official resources because then there would be too many. ccw made a load of shaders like detail_shader that makes the game look better but didn't add them to official resources and instead they are on community.mtasa.com

I suppose the difference with this one is because it fits with the base game in such a non intrusive way that it could be a vanilla feature, but that's not my call at the end of the day.
Fernando-A-Rocha reacted with thumbs up emoji

Copy link
Contributor

ArranTuna commented Sep 21, 2025 •
edited
Loading

Although it looks good in the video I don't think it should be added to the official resources for the same reason all the other amazing scripts that are on community.mtasa.com are not official resources because then there would be too many. ccw made a load of shaders like detail_shader that makes the game look better but didn't add them to official resources and instead they are on community.mtasa.com

I suppose the difference with this one is because it fits with the base game in such a non intrusive way that it could be a vanilla feature, but that's not my call at the end of the day.

I suppose it is only 92 KB and some maps are as big as that. So I think adding it would be fair, if you really can just do 'start ladders' and all the ladders in the default map become functional, then that is useful.

I wouldn't want to review the PR though because I really don't like looking at cramped up stuff like blah==true

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

Reviewers

1 more reviewer

@Nico8345 Nico8345 Nico8345 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /