test.patch1 KBDownload
Details
Details
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 9786 Build 16459: arc lint + arc unit
Event Timeline
| source/gui/CGUI.cpp | ||
|---|---|---|
| 892–899 | Something similar can be triggered for this case:
vfs_lookup.cpp(141): Assertion failed: "!pathname.IsDirectory()" Assertion failed: "!pathname.IsDirectory()" Location: vfs_lookup.cpp:141 (vfs_Lookup) The rationale is that one can pass any JS and XML to the engine but won't be able to crash it. | |
| 908–915 | Done in rP23068 | |
Comment Actions
See PM discussion yesterday, this will only catch the case where the directory doesnt end with a slash, but in case someone has a typo in the foldername, it will not report that and continue like nothing happened, just with follow up errors.
There was also a discussion about the ENSURE in VfsDirectoryExists from rP16773.
An ENSURE means if the condition is not met, it is an error. by the C++ author. If it is an error by the JS or XML author, then it should use LOGERROR or JS_ReportError.
It appears the VfsDirectoryExists function is currently only used for hardcoded folders. This would be the first use case where user-input (JS/XML) is fed.
As nani indicated, the ENSURE may be moved to the callers of that function too.
(One could also wonder whether its a feature that 0 files being loaded is going unreported, so that mods could delete entire directories without overwriting the directory including including XML pagefile.
But usually if they want to delete a folder, they have to change much more already. And the XML files should also be split so that changing the includes is more friendly for combining mods (for example 0ad + fgod + autociv).)
Comment Actions
The patch results in
ERROR: GUI: Script path gui/pregame is not a directory
while it actually is a directory, just not one recognized by the code.
At least from a perspective of a modder who doesnt know anything about the C++ code, he will wonder whats wrong, and doesnt get a clue that the trailing slash is missing.
I will add the word "path", hopefully making that more clear, as I think the addition of "did you forget to add a /?" would give a wrong hint in case there was a typo for example or a filepath given.
ERROR: GUI: Script path gui/pregame is not a directory path
Notice that this function actually only checks whether the string is nonempty and ends with slash, but doesnt check if its a directory or file nor if it exists.
The latter cases are covered by a VFS logerror, therefore the patch is complete.
The file check is useful because it means a logerror is shown instead of the application crashing / running into a debug breakpoint.
Thanks for the patch nani.