search

pBlueG / SA-MP-MySQL

MySQL plugin for San Andreas Multiplayer
BSD 3-Clause "New" or "Revised" License
194 stars 81 forks source link

Totally untested sscanf utilising code... #240

Open Y-Less opened 3 years ago

Y-Less commented 3 years ago

I'm making this PR now as a work tracking PR (because hacktoberfest; if you could add the tag, that would be awesome). I've not tested most of it yet because I can't actually build the plugin (#239), but I know the sscanf lookup and RPC works thanks to other testing (Y-Less/micro). This adds a new function:

native cache_get_native(cache, row, const format[], {Float, _}:...);

This is basically a wrapper around sscanf that passes the row data directly in and allows you to parse it with an sscanf-compatible specifier string. It actually looks for and uses the sscanf plugin (sscanf needs to come first), so there's very little overhead in the code, and if you don't want it just don't include both plugins.

maddinat0r commented 3 years ago

Thanks for the PR! I'm not the owner of this repository (sad, I know), so I can't add the hacktoberfest topic. However, if I understood it correctly, adding the "accepted" label should suffice. I've linked a rough pointer to build the plugin in that issue you created, but if you have more specific questions, feel free to ask.

Y-Less commented 3 years ago

I saw the link, thanks. I'm working on building/testing it now (got past the cmake stage, so making progress). Any comments on the general idea?

maddinat0r commented 3 years ago

I'm all for it, as long as it's not too intrusive (which it isn't).
One thing I just noticed that I'd change is the log messages. You're currently printing a "failed" log message when the sscanf plugin is not found or has an invalid version, which users might interpret as an error message. I'd remove those completely (fail silently) and only print a log message if the sscanf function has been found successfully.

Y-Less commented 3 years ago

I'll move those to the function itself - not finding the plugin is fine, unless you try calling the function. Actually, the code will not register the native if the sscanf plugin doesn't exist, because you can't call it, but then a user trying to call it without sscanf will get a "function not found" error, despite having the correct version loaded. That's even more confusing, so I could try check which natives they need from the AMX and print a more useful error if it is found in the header.