search

multitheftauto / amx

MTA AMX compatibility layer.
zlib License
31 stars 8 forks source link

scriptfiles path is incorrect #47

Open Disinterpreter opened 4 years ago

Disinterpreter commented 4 years ago

Describe the bug There is a problem with scriptfiles. Probably my problem is bad reading of the documentation. But I can't work with scriptfiles.

I tried to put scriptfiles into the amx/scriptfiles, and amx-mygamemode/scriptfiles I tried to move amx into /resources/amx and put file there. But all my moves didn't work.

To reproduce Just try to read data from scriptfiles. Or try to launch something like https://github.com/Open-GTO/Open-GTO

Expected behaviour It must work fine. The files from script files must be available.

Additional context https://github.com/multitheftauto/amx/blob/1cb22d58995d1871006ab6a8b3c2099f532f6e1b/amx-deps/src/util.cpp#L207-L219

colistro123 commented 4 years ago

The main problem seems to be that the getScriptFilePath function isn't referenced anywhere except for sqlite reading stuff, so the path is probably never set.

Disinterpreter commented 4 years ago

The main problem seems to be that the getScriptFilePath function isn't referenced anywhere except for sqlite reading stuff, so the path is probably never set.

https://github.com/Disinterpreter/pmta-wrapper/search?q=getScriptFilePath&unscoped_q=getScriptFilePath

I think it can help. It is latest version adamix's AMX.

qaisjp commented 4 years ago

You may find it useful to incorporate the fix from 1325abf20ceab228e89c21d041849742dcc6d5bb

theSpool commented 4 years ago

Ah, it's true, AMX has that SA-MP specific change, I just forgot about it when changing the AMX version. I'll have to put it back in.

theSpool commented 4 years ago

OK, there's a small inconsistency with SA-MP in the previous AMX. SA-MP uses a single path for the scriptfiles of every AMX script, and, for security reasons (to avoid malicious compiled scripts touching the rest of the file system), the natives aren't allowed to go outside this folder. This module's implementation looks on both for an existing file before defaulting to the script's file.

Although at first glance SA-MP's approach might seem problematic, it's been the standard an some scripts might rely on it. A script might create a file or database and expect another script to use its contents. With this resources implementation, this wouldn't work.

I looked into the current file natives implementation and I noticed it uses a completename function which looks for the env var AMXFILE_VAR (which currently is defined as "AMXFILE"), which can be specified by the user. I'd suggest to use setenv while initializing the module with the overwrite value set to false so users can set it to whatever they want. This would implement and extend (rather than change) SA-MP's behaviour.

Any comments?

colistro123 commented 4 years ago

Thanks @theSpool, resolved in https://github.com/multitheftauto/amx/commit/422b841d8f07a43d56a5463a56ef151e91bc55f5#diff-f5ac81a5af817525061fa6eafe4ca6eaR140

If you have any better solutions, such as this, feel free to add them I just noticed that, my bad:

I'd suggest to use setenv while initializing the module with the overwrite value set to false so users can set it to whatever they want. This would implement and extend (rather than change) SA-MP's behaviour.

Disinterpreter commented 4 years ago

And where the scriptfiles folder must be now?

qaisjp commented 4 years ago

It's unfortunately still hardcoded to resources/amx/scriptfiles. So if your amx resource is in a different folder (likely), it won't work.

Disinterpreter commented 4 years ago

It's unfortunately still hardcoded to resources/amx/scriptfiles. So if your amx resource is in a different folder (likely), it won't work.

I don't know what I do incorrect. I build from latest commit. I put scriptfiles. And got an error. (I trying to launch OpenGTO)