superman: more initial improvements #577
Conversation
| <script src="config/ShSupermanConfig.lua" type="shared" cache="false"/> | ||
| <script src="data/ShData.lua" type="shared" cache="false"/> | ||
| <script src="data/CData.lua" type="client" cache="false"/> | ||
| <script src="data/SData.lua" type="server"/> | ||
|
|
||
| <script src="CHandleSuperman.lua" type="client" cache="false"/> | ||
| <script src="SHandleSuperman.lua" type="server"/> |
There was a problem hiding this comment.
I don't think cache="false" is wanted for public official resources.
There was a problem hiding this comment.
It's used, e.g in admin and internetradio
There was a problem hiding this comment.
I believe it is (and should) be used in critical scripts to harden security, but I'm not sure this resource qualifies. It would be better to allow caching to reduce bandwidth/download size.
| -- Static global variables | ||
| local thisResource = getThisResource() | ||
| -- Static variables | ||
|
|
||
| local serverGravity = getGravity() |
There was a problem hiding this comment.
This variable can be named with capital letters as it's static and is not updated in the code, like the settings (it's a constant). This is personal preference / good practice, that I recommend.
There was a problem hiding this comment.
I only use upper case for global variables/tables. This is clear sign that you're dealing with global and not local.
| setElementRotation(playerElement, 0, 0, 0) | ||
| setElementCollisionsEnabled(playerElement, true) | ||
| self.rotations[playerElement] = nil | ||
| self.previousVelocity[playerElement] = nil | ||
| end | ||
|
|
||
| function angleDiff(angle1, angle2) |
There was a problem hiding this comment.
This function can be made local as it's only used in this scriptfile.
| self:restorePlayer(playerElement) | ||
| end | ||
| end | ||
|
|
||
| -- onEnter: superman cant enter vehicles | ||
| -- There's a fix (serverside) for players glitching other players' vehicles by warping into them while superman is active, causing them to flinch into air and get stuck. | ||
| function showWarning() |
There was a problem hiding this comment.
Please assure function scopes are as optimized as possible. This one, and other Superman:... functions can be made local, afaik
There was a problem hiding this comment.
I don't really want touch those for now, if anything i wanted to fix it in next PR, to get rid of table approach.
There was a problem hiding this comment.
Alright sur for a future PR
[gameplay]/superman/data/SData.lua
Outdated
| addEvent("onServerSupermanSetData", true) | ||
| addEventHandler("onServerSupermanSetData", root, onServerSupermanSetData) | ||
|
|
||
| function onPlayerResourceStartSyncSuperman(startedResource) |
There was a problem hiding this comment.
This func can be local
There was a problem hiding this comment.
I don't over localize anything possible, only helper functions which aren't used besides certain .lua file. Do note that Lua has hardcoded limit of 200 local variables and 60 upvalues. But will change it.
There was a problem hiding this comment.
It's fine to localize within this script only the functions that u won't use outside of it, staying way below the limit. It's not exaggeration
[gameplay]/superman/data/SData.lua
Outdated
| end | ||
| addEventHandler("onPlayerResourceStart", root, onPlayerResourceStartSyncSuperman) | ||
|
|
||
| function onPlayerQuitClearSupermanReceiver() |
There was a problem hiding this comment.
This func can be local
[gameplay]/superman/data/CData.lua
Outdated
| @@ -0,0 +1,11 @@ | |||
| function onClientSupermanSync(supermanServerData) | |||
There was a problem hiding this comment.
These funcs can be local
|
Mostly good now 👍🏼 |
-> localPlayer can now only change (force sync to server) it's own data
-> Ensure valid dataKey and dataValue, by checking whether they match types (string & boolean), and allowed key names