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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

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 if was checking MTASA_VERSION_TYPE which 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 #3853 (comment)).

Also, I changed the Resource Upgrade feature to create min_mta_version 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.

Test

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

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

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.

3 participants