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

Replaced default CLEO directory in start script opcodes. #187

Closed
wants to merge 1 commit into from

Conversation

MiranDMC
Copy link
Collaborator

No description provided.

@MiranDMC MiranDMC requested a review from x87 September 12, 2024 20:35
@x87
Copy link

x87 commented Sep 13, 2024

@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
Copy link

x87 commented Sep 13, 2024

Can we update ResolvePath in such a way that it:

  1. Resolves the absolute path:
  • if input path starts with a virtual prefix
    • resolve the prefix path and
      • set absolutePath to prefix path + input path
  • else if input path is absolute:
    • set absolutePath to input path
  • else if customworkdir has been given:
    • resolve customworkdir and
      • set absolutePath to customworkdir path + input path
  • else if workdir has been set with 0A99:
    • resolve script workdir and
      • set absolutePath to workDir path + input path
  • else:
    • set absolutePath to game root path + input 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
Copy link
Collaborator Author

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:
#143
Which was made by adding ML support to already existing void CLEO_ResolvePath and CLEO_ListDirectory exports.

@x87
Copy link

x87 commented Sep 13, 2024

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

@MiranDMC
Copy link
Collaborator Author

MiranDMC commented Sep 13, 2024

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
Copy link

x87 commented Sep 13, 2024

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
Copy link
Collaborator Author

MiranDMC commented Sep 13, 2024

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
Copy link

x87 commented Sep 13, 2024

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

@MiranDMC
Copy link
Collaborator Author

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
Copy link

x87 commented Sep 14, 2024

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
Copy link
Collaborator Author

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.

@MiranDMC MiranDMC closed this Sep 16, 2024
@MiranDMC MiranDMC deleted the ModLoader_fixes branch September 20, 2024 00:50
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.

2 participants