Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MinClientReqCheck and improve resource upgrade #3862

Merged
merged 8 commits into from
Nov 21, 2024

Conversation

Fernando-A-Rocha
Copy link
Contributor

@Fernando-A-Rocha Fernando-A-Rocha commented Nov 19, 2024

Details

  • I found that MinClientReqCheck was not working on any builds because it was checking MTASA_VERSION_TYPE, which was not defined due to a missing version.h import. I have restored this include in server StdInc.h, as it was back in the day (before a big refactor that removed this by mistake, see Fix min_mta_version 'server' not working as expected #3853 (comment)). This was resulting in clientside functions like fetchRemote which use old syntax not outputting any warnings.

  • I changed the Resource Upgrade feature to set the min_mta_version meta.xml node with the "both" attribute when the client & server versions to set are the same (CResourceChecker.cpp). Attribute "both" is already correctly supported on resource parsing/loading.

  • I changed the way base64Encode and base64Decode functions are detected by the resource checker (& upgrader). Previously, the upgrader would automatically rename these functions in your scripts to encodeString and decodeString, causing the scripts to break because these functions take different arguments (different syntaxes). Now, instead of doing that automatically, it warns you to manually update, like it does for other functions such as givePlayerJetPack.

Tests

min_mta_version attribute "both"

Note

Tested by commenting the MTASA_VERSION_TYPE checks at MinServerReqCheck and MinClientReqCheck so this can run on custom builds:

Place this code on client.lua and server.lua script files in a new resource.

fileGetContents("doesnt_matter")

Start the server to load all resources or start the resource if the server was already running.

The resource upgrader will detect the missing min_mta_version information due to this new 1.6.0 function being used, which was added in 1.6.0-9.21938 for both client and server.

The upgrade command will result in the meta.xml being successfully updated with the new "both" min_mta_version required.

image

base64Encode and base64Decode detection

Place this code in a serverside script (taken from the wiki).

local k = base64Encode("Hello, world!")
outputServerLog(k)
--Output: SGVsbG8sIHdvcmxkIQ==

Start the resource. We now observe a warning asking you to update the function manually, where previously it would wrongfully replace the function name (base64Encode) with encodeString which has a different syntax.

WARNING: testres/server.lua(Line 1) [Server] base64Encode no longer works. Please manually change this to encodeString (different syntax). Refer to the wiki for details

Copy link
Contributor

@Nico8340 Nico8340 left a comment

Choose a reason for hiding this comment

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

Perfect so far!

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Nov 19, 2024

Some facts

Regarding resource upgrader:

I found that the upgrade command can only upgrade resources by setting a new min_mta_version client/server setting via the CResourceChecker::CheckVersionRequirements function that depends on client and server min versions listed in CResourceChecker.Data.h

image

Other older warnings like fetchRemote, callRemote etc cannot be automatically resolved with upgrade

They only pollute the debug console until the developer manually does something about the script(s).

image

@Fernando-A-Rocha Fernando-A-Rocha marked this pull request as ready for review November 19, 2024 15:44
@Fernando-A-Rocha
Copy link
Contributor Author

Perfect so far!

I'm gonna look into what you said on discord:

image

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Nov 20, 2024

@Nico8340 Ive committed a solution for base64Encode and base64Decode.

Main PR post updated.

@Dutchman101
Copy link
Member

Nice

@Dutchman101 Dutchman101 merged commit f095410 into multitheftauto:master Nov 21, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants