Skip to content

Conversation

ghost
Copy link

@ghost ghost commented May 29, 2021

I made utils functions to return a boolean, this function is checking whether the feature is active or not, so it will return a false or a true one.
Why did you do that?
Well, when you go to the "Maps" tab, a variable will return an error, if the resource is not linked to the variable and leading to null within ipairs.

I made utils functions to return a boolean, this function is checking whether the feature is active or not, so it will return a false or a true one.
Why did you do that?
Well, when you go to the "Maps" tab, a variable will return an error, if the resource is not linked to the variable and leading to null within ipairs.
@stoneage-mta
Copy link
Contributor

stoneage-mta commented May 30, 2021

Well, thank you for the pull request, the idea is very good and I also have this same problem of ending up stopping the resource mapmanager and having this annoying error on the server console, but I think the implementation you applied could be much simpler and more advantageous, without having to add any new functions and also preventing these multiple calls to getResourceFromName in the function getServerMaps.

And another thing, within the original code, there is already a check if the resource exists, and if it does not exist it sends a trigger to the client with an empty table as result, and with a comment of "fake data". This behavior shouldn't be expected if the resource exists but is not being executed? (I didn't dive into the original code to find out if the result is really expected to be achieved - even if it is "fake")

Copy link
Contributor

@stoneage-mta stoneage-mta left a comment

Choose a reason for hiding this comment

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

there are still a few other places (lines: 34, 43, 57 and 58) to replace getResourceFromName("mapmanager") to the mapmanager variable, but for some reason, i cant select them to comment on)

@stoneage-mta
Copy link
Contributor

Now it looks good, maybe we should check if the same bug exists in admin2?

@ghost
Copy link
Author

ghost commented Jun 1, 2021

Now it looks good, maybe we should check if the same bug exists in admin2?

I'll just wait.

Copy link
Contributor

@stoneage-mta stoneage-mta left a comment

Choose a reason for hiding this comment

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

I just checked and the admin2 resource doesn't use mapmanager anywhere.

@jlillis jlillis merged commit 0f4ef32 into multitheftauto:master Jun 7, 2021
@patrikjuvonen patrikjuvonen added this to the 1.5.9 milestone Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants