Add error codes to exceptions, C bindings, rename python libs

[gtker]

Hey there again, got the initial C bindings ready.

In order to get error reporting beyond just an exception has occurred I needed to attach integer values to the exceptions that were previously just strings. I considered just comparing the string with what I knew it would be, but it seems like the strings are different for Linux/Windows, so that wouldn't work out.

I realize that changing the exceptions is a pretty major thing if it doesn't fit into your vision, so just let me know and I'll try to figure something else out.

I don't have a great subject matter understanding of the library, so the error codes are basically just the error messages with underscores.

I also changed the python libraries to have _python in them to make them stand out. I considered naming the C/C++ libraries with _c but considered it a bit weird.

Lastly, I needed a way to not install the 20MB test file, since it won't get uploaded to the crates.io Rust dependency repository.

Let me know if you want any changes.

Additionally, I wanted to ask what the intended usage of BuildMap is. The function requires a map name and I don't see a way of auto exporting all maps in the entire MPQ. Am I just supposed to list out all individual maps or am I missing something?

[gtker]

Didn't rename the Python module imports. Rebased the changes and pushed a new branch.

[gtker]

Reverted the Python changes since they seem to be more problems than they're worth.

[gtker]

Sorry to run up your CI minutes, but I had trouble with non utility::exception exceptions being thrown and then SIGABRTing the Rust binary. This should just blanket catch all exceptions and then sort them out later.

[namreeb]

I realize that changing the exceptions is a pretty major thing if it doesn't fit into your vision, so just let me know and I'll try to figure something else out.

I'm fine with the change you've made. The reason I went with exceptions to begin with was to allow myself the freedom to parameterize the strings. This would allow for a client of the library to spit out something like "Generation of ADT Azeroth (32, 48) failed." rather than "Generation of some ADT failed".

Reverted the Python changes since they seem to be more problems than they're worth.

Probably for the best. I wouldn't want to require users of the Python library to have to do import pathfind_python.

Sorry to run up your CI minutes, but I had trouble with non utility::exception exceptions being thrown and then SIGABRTing the Rust binary. This should just blanket catch all exceptions and then sort them out later.

Hmm, this is surprising to me. Do you know how to trigger exceptions which are not utility::exception? If that is in my code then that should get fixed. If it is in code from a dependency then namigator should probably catch it and throw a utility::exception instead. The goal is for the internals of the library to be opaque and leaking unexpected exceptions is not very user-friendly.

[gtker]

Do you know how to trigger exceptions which are not utility::exception?

The specific exception that lead me to include the catch (...) was attempting to std::fs::directory_iterator over a directory in MpqManager::Initialize that didn't exist, which threw a std::filesystem::filesystem_error.

[namreeb]

Additionally, I wanted to ask what the intended usage of BuildMap is. The function requires a map name and I don't see a way of auto exporting all maps in the entire MPQ. Am I just supposed to list out all individual maps or am I missing something?

The intended map building process looks like this:

1. Generate BVH data for all objects. There is a mapbuilder runtime argument to do this though I don't remember what it was, maybe "-b"?
2. For each desired map, run "mapbuilder -m map_name".

So yes, the user is supposed to give a list of the maps they want to build. This is because there are an awful lot of maps that you probably don't care about building data for (and may not even parse correctly).

Incidentally if you have other questions about the library you're welcome to add me on discord: namreeb#2747.

The specific exception that lead me to include the catch (...) was attempting to std::fs::directory_iterator over a directory in MpqManager::Initialize that didn't exist, which threw a std::filesystem::filesystem_error.

Okay, I pushed a fix for this with commit 9d442d3. I'd prefer that you either drop the catch (...) or replace it with something loud to motivate people to report any other exception leaking that may occur in the future.

[gtker]

This is because there are an awful lot of maps that you probably don't care about building data for (and may not even parse correctly).

Is this referring to random cruft maps that are inaccessible in game, or just maps that usually doesn't have mobs on them like the Developer Island and Emerald Dream maps?

Incidentally if you have other questions about the library you're welcome to add me on discord: namreeb#2747.

Added you.

I'd prefer that you either drop the catch (...) or replace it with something loud to motivate people to report any other exception leaking that may occur in the future.

If it's OK with you I would like to prefer having the catch (...) around FFI boundaries because if an exception occurs (at least for Rust bindings) the entire program just shuts down without any trace info or message beyond "Exceptions can't cross FFI boundaries" or something like that. It would be significantly easier to figure out what went wrong if I can panic from Rust and add additional context. I promise to make any UNKNOWN_EXCEPTION occurrences panic so that users can't just ignore them.

[namreeb]

Is this referring to random cruft maps that are inaccessible in game, or just maps that usually doesn't have mobs on them like the Developer Island and Emerald Dream maps?

It refers to all maps. Azeroth, Kalimdor, and even PvPZone04! I believe they are enumerated in one of the DBCs.

If it's OK with you I would like to prefer having the catch (...) around FFI boundaries because if an exception occurs (at least for Rust bindings) the entire program just shuts down without any trace info or message beyond "Exceptions can't cross FFI boundaries" or something like that. It would be significantly easier to figure out what went wrong if I can panic from Rust and add additional context. I promise to make any UNKNOWN_EXCEPTION occurrences panic so that users can't just ignore them.

Okay, I'll accept that.