Lots of fixes and improvements #98
Conversation
also returned SetPlayerDisabledWeapons deprecation, many fixes to string length return in functions
and add a couple of earlier unimplemeted functions
No more "Sorry, but [native] is not implemented" when the server start, now all dummies are included and some of previously unimplemented dummies are implemented into working functions
and also new natives from MTA in a_amx.inc
by changing them to native bit* functions
obtain announce and lanmode correctly
There was a problem hiding this comment.
I have reviewed and tested the changes, and overall, they look good. I have left a few comments on some code I'd like to see adjusted.
For future PRs, it would be helpful if they were broken down a little bit more. This makes the review process more efficient for the person that has to go through it. I also noticed a couple of redundant elements, which add unnecessary noise to the review (I commented on those so said things can be avoided in the future).
Overall, creating smaller PRs makes it easier to spot issues and revert changes when needed, while also allowing other contributions to be merged more quickly. As the saying goes, it’s best not to put all your eggs in one basket.
Have a great weekend and thanks for your awesome contribution! 😊
| { x = 0, y = 0, z = 0, c = tocolor(0, 0, 0, 255) }, | ||
| { x = 0, y = 0, z = 0, c = tocolor(0, 0, 0, 255) }, | ||
| { x = 0, y = 0, z = 0, c = tocolor(0, 0, 0, 255) }, | ||
| { x = 0, y = 0, z = 0, c = tocolor(0, 0, 0, 255) } |
There was a problem hiding this comment.
For now we can leave this, but I will advise against doing changes like these in the future. It simply creates noise, causing the review process to be lengthier.
| addEvent('OnPlayerGiveDamageActor_Ev', true) | ||
| addEventHandler('OnPlayerGiveDamageActor_Ev', root, | ||
| function(actor, loss, weapon, bodypart) | ||
| if not actor or getElementData(actor, 'amx.invulnerable') then return end |
There was a problem hiding this comment.
This was changed to 'Invulnerable' via setElementData, meaning accessing it this way will most likely not work.
| GetVehicleModelInfo = {'i', 'i', 'r', 'r', 'r'}, | ||
| GetVehicleParamsCarDoors = {'v', 'r', 'r', 'r', 'r'}, | ||
| GetVehicleParamsEx = {'v', 'r', 'r', 'r', 'r', 'r', 'r', 'r'}, | ||
| GetVehicleParamsSirenState = {'v'}, | ||
| GetVehiclePoolSize = {}, |
There was a problem hiding this comment.
As I said for other things in my previous comment, let's avoid moving things like these in the future, there's no need to do so. 😊
| playerdata.returntoclasssel = nil | ||
| putPlayerInClassSelection(player) | ||
| return true | ||
| else | ||
| spawnPlayerBySelectedClass(player) |
There was a problem hiding this comment.
If TogglePlayerSpectating is called too early (when loading player's data in order to spawn them) certain scripts might misbehave and cause the player to be kicked when OnPlayerSpawn kicks in (especially if there's checks to see if they have logged in and this executes prior to loading the player's data). Additionally, I believe this shouldn't be here.
I have tested this and it works fine for my specific use-case, it replicates just what SA-MP does.
| spawnPlayerBySelectedClass(player) | |
| --spawnPlayerBySelectedClass(player) |
| spawnPlayerBySelectedClass(player) | ||
| --In samp calling TogglePlayerSpectating also unsets camera interpolation | ||
| clientCall(player, 'removeCamHandlers') | ||
| setElementAlpha(player, 255) |
There was a problem hiding this comment.
Up above we're disabling all controls when spectating, but never re-enabling them.
I also add PLAYER_STATE_NONE to mirror OpenMP's behavior to some degree.
| setElementAlpha(player, 255) | |
| setElementAlpha(player, 255) | |
| toggleAllControls(player, true, true, false) | |
| setPlayerState(player, PLAYER_STATE_NONE) |
| if g_Players[playerID].doingclasssel then | ||
| return | ||
| end |
There was a problem hiding this comment.
If we're spectating don't send an event to show the UI. I believe this is what SA-MP does and it mirrors my gamemode's behavior in SA-MP (I tested this).
| if g_Players[playerID].doingclasssel then | |
| return | |
| end | |
| if g_Players[playerID].doingclasssel or g_Players[playerID].state == PLAYER_STATE_SPECTATING then | |
| return | |
| end |
| oX = 0.0, oY = 0.0, oZ = 0.0, | ||
| hX = 0.0, hY = 0.0, hZ = 0.0 | ||
| } | ||
| g_Players[playerID].conntick = getTickCount() | ||
| g_Players[playerID].viewingintro = true |
There was a problem hiding this comment.
Let's also initialize the state as PLAYER_STATE_NONE.
| g_Players[playerID].viewingintro = true | |
| g_Players[playerID].viewingintro = true | |
| g_Players[playerID].state = PLAYER_STATE_NONE |
GetConsoleVarAs*pawn natives) updated for actual SA-MP 0.3.7 version list. They now also sync with MTA config settings where it's possible (where they fully match, like MTA'sase-> SA-MP'sannounce) and integrated with meta.xml or settings.xml, which means you can obtain any custom config variables from those config files withGetConsoleVarAs*functions (if a console variable isn't found in default variables list, it will try to get it from config files). New ACL right requests for "function.getServerPassword" and "function.setServerPassword" were added because more rcon variables (includingpassword) were implemented, and it's now also noted in README.mdscoreboarddefault resource which is now correctly validated if it's running and only then it's used)getNetworkStatsinfo)~g~,~r~,~y~etc)WEAPONSTATE_RELOADINGwhen actual reloading (#82)getPedRotationfunction, moreover doing it incorrectlybinor,binand,binshrandbinshlreplaced with built-in and more relevant bitOr, bitAnd, bitRShift and bitLShift