cleolibrary / CLEO5

CLEO 5 for GTA San Andreas
https://cleo.li
MIT License
55 stars 6 forks source link

Replaced default CLEO directory in start script opcodes. #187

Closed MiranDMC closed 2 months ago

x87 commented 2 months ago

@MiranDMC

https://github.com/cleolibrary/CLEO5/blob/9670d556b8a59f73432348746588ad8bcbb041c1/source/CScriptEngine.cpp#L719C28-L719C44

weakly_canonical returns an absolute path, so ResolvePath("cleo") / fsPath makes the path absolute.

x87 commented 2 months ago

Can we update ResolvePath in such a way that it:

1) Resolves the absolute path:

  1. Before returning:
    • if the absolute path starts with the usersettings directory:
    • return absolutePath
    • else calculate a new path RELATIVE to the game directory (relativePath) and
    • if relativePath is not calculated:
    • throw an error
    • else:
    • return relativePath

Test cases

root = C:\root user = C:\userfiles cwd = C:\root

  1. ResolvePath("test.cs") -> "test.cs"
  2. ResolvePath("..\test.cs") -> error
  3. ResolvePath("test.cs", "cleo:") -> "cleo\test.cs"
  4. ResolvePath("cleo:\test.cs") -> "cleo\test.cs"
  5. ResolvePath("root:\test.cs", "cleo:") -> "test.cs"
  6. ResolvePath("test.cs", "user:") -> "C:\userfiles\test.cs"
  7. ResolvePath("user:\test.cs") -> "C:\userfiles\test.cs"
  8. ResolvePath("test.cs", "modules:") -> "cleo\cleo_modules\test.cs"
  9. ResolvePath("test.cs", "root:") -> "test.cs"
  10. ResolvePath("C:\root\test.cs") -> "C:\root\test.cs"
  11. ResolvePath("C:\userfiles\test.cs") -> "C:\userfiles\test.cs"
  12. ResolvePath("C:\foo\test.cs") -> error

0A99: 0

same results as above

0A99: 1

  1. ResolvePath("test.cs") -> "C:\userfiles\test.cs"
  2. ResolvePath("..\test.cs") -> error
  3. ResolvePath("test.cs", "cleo:") -> "cleo\test.cs"
  4. ResolvePath("cleo:\test.cs") -> "cleo\test.cs"
  5. ResolvePath("root:\test.cs", "cleo:") -> "test.cs"
  6. ResolvePath("test.cs", "user:") -> "C:\userfiles\test.cs"
  7. ResolvePath("user:\test.cs") -> "C:\userfiles\test.cs"
  8. ResolvePath("test.cs", "modules:") -> "cleo\cleo_modules\test.cs"
  9. ResolvePath("test.cs", "root:") -> "test.cs"
  10. ResolvePath("C:\root\test.cs") -> "C:\root\test.cs"
  11. ResolvePath("C:\userfiles\test.cs") -> "C:\userfiles\test.cs"
  12. ResolvePath("C:\foo\test.cs") -> error

0A99: "data"

  1. ResolvePath("test.cs") -> "data\test.cs"
  2. ResolvePath("..\test.cs") -> "test.cs"
  3. ResolvePath("test.cs", "cleo:") -> "cleo\test.cs"
  4. ResolvePath("cleo:\test.cs") -> "cleo\test.cs"
  5. ResolvePath("root:\test.cs", "cleo:") -> "test.cs"
  6. ResolvePath("test.cs", "user:") -> "C:\userfiles\test.cs"
  7. ResolvePath("user:\test.cs") -> "C:\userfiles\test.cs"
  8. ResolvePath("test.cs", "modules:") -> "cleo\cleo_modules\test.cs"
  9. ResolvePath("test.cs", "root:") -> "test.cs"
  10. ResolvePath("C:\root\test.cs") -> "C:\root\test.cs"
  11. ResolvePath("C:\userfiles\test.cs") -> "C:\userfiles\test.cs"
  12. ResolvePath("C:\foo\test.cs") -> error

0A99: "..\root\data"

same results as above

0A99: ".."

  1. ResolvePath("test.cs") -> error
  2. ResolvePath("..\test.cs") -> error
  3. ResolvePath("test.cs", "cleo") -> "cleo\test.cs"
  4. ResolvePath("cleo:\test.cs") -> "cleo\test.cs"
  5. ResolvePath("root:\test.cs", "cleo:") -> "test.cs"
  6. ResolvePath("test.cs", "user:") -> "C:\userfiles\test.cs"
  7. ResolvePath("user:\test.cs") -> "C:\userfiles\test.cs"
  8. ResolvePath("test.cs", "modules:") -> "cleo\cleo_modules\test.cs"
  9. ResolvePath("test.cs", "root:") -> "test.cs"
  10. ResolvePath("C:\root\test.cs") -> "C:\root\test.cs"
  11. ResolvePath("C:\userfiles\test.cs") -> "C:\userfiles\test.cs"
  12. ResolvePath("C:\foo\test.cs") -> error
MiranDMC commented 2 months ago

This is writing explicit ModLoader support, but embedded into fabric of whole internal CLEO logic. There is difference between preparing file path before using it somewhere in hope ML picks it up with hooked APIs and resolving file paths for other purposes.

As proposed before whole path processing inside CLEO should be done without complicating it with ML peculiarities, then just translate resulting path to the ML standard just before using it. However that solution is also weak as apparently ML is not consistent in it's working; some APIs work with absolute paths, other require relatives, returned results also vary. Which mean every single place where path is send to some API it need to take in consideration how ML is handling it and how the path should be preprocessed.

CLEO5 was already rewritten to use absolute paths internally to avoid any ambiguities. Once resolved path is absolute and finite. Any further resolving causes no change. Proper Mod Loader was implemented in branch: https://github.com/cleolibrary/CLEO5/pull/143 Which was made by adding ML support to already existing void CLEO_ResolvePath and CLEO_ListDirectory exports.

x87 commented 2 months ago

There is no new logic in the examples, it's just structured + tests cases are given. Which of them you think are wrong?

MiranDMC commented 2 months ago

There is no new logic in the examples, it's just structured + tests cases are given. Which of them you think are wrong?

Resolving cleo: into cleo is not resolving anything at all.

0A99 argument is not something that is stored anywhere and can be used for test cases in examples. Only persisting result is actual work dir. So before processing any path it should be resolved to absolute.

0A99 "root:" // or actual non virtual
open_file "cleo\test.ini"

and

0A99 "cleo:" // or actual non virtual
open_file "test.ini"

should produce same result, so there is no point for checking what initial ingredients were.

So if preparing path for ML it would make sense to check if the resulting path starts with game directory, then cut game directory prefix from it. Then also it should be checked if global work dir is actually set to game dir as ML depends on it.

Then it all gets entangled again when taking into account situation where find_first_file is used along with 0A99

All those problems are already solved with generic approach and ML plugin providing exports: give me file, list me files. Just do what you need to do instead of playing around and pretending fake paths.

x87 commented 2 months ago

What test case I provided is incorrect?

0A99 argument is not something that is stored anywhere and can be used for test cases in examples.

it is stored as script's workdir and it is used in the logic I described as workdir

else if workdir has been set with 0A99:

    resolve script workdir and
        set absolutePath to workDir path + input path
MiranDMC commented 2 months ago

They seems to be logically ok, in matter of what you want to send to ML. As mentioned CLEO5 is intentionally not working on relative paths internally, so that logic can not be embedded in generic util function for resolving paths.

But... "logically" is not what ML is doing, as absolute/relative paths inconsistency was already observed.

x87 commented 2 months ago

What functionality you expect to break if ResolvePath implementation changes to the proposed one?

MiranDMC commented 2 months ago

What functionality you expect to break if ResolvePath implementation changes to the proposed one?

resolve_filepath opcode returning not resolved paths/mixed styles, cleo.log reporting ambiguous paths and potentially any operation in core or plugins that depended on fact resolved path is absolute.

x87 commented 2 months ago

resolve_filepath opcode returning not resolved paths/mixed styles, cleo.log reporting ambiguous paths

currently they return/report wrong paths assuming one location (CLEO) where in fact the physical location of the file is different (modloader). I think it is equally bad.

MiranDMC commented 2 months ago

resolve_filepath opcode returning not resolved paths/mixed styles, cleo.log reporting ambiguous paths

currently they return/report wrong paths assuming one location (CLEO) where in fact the physical location of the file is different (modloader). I think it is equally bad.

I think it is already solved in ML branch. Resolving CLEO path is not so trivial, as each autostarted custom script believe it is located inside CLEO. To complicate things ML loads every cs file it find in any subdirectory of mod.