openplanet-nl / issues

Issue tracker for Openplanet.
10 stars 0 forks source link

better deprecation warnings #493

Closed ezio416 closed 4 months ago

ezio416 commented 4 months ago

With the new Text formatting functions and the deprecation of the old global ones, we are getting errors about these old functions being deprecated without any information on where in a plugin's code they may be. These warnings are also only given when the old functions are called (through Compatibility.as), so they do not necessarily catch every instance in a plugin. It would instead be nice for Openplanet itself to give warnings about this type of thing, similar to the signed/unsigned mismatch warnings.

XertroV commented 4 months ago

Currently compatibility.as reports it like this:

namespace Internal
{
    bool ReportedGlobalTextConversionDeprecation = false;
    void ReportGlobalTextConversionDeprecation()
    {
        if (ReportedGlobalTextConversionDeprecation) {
            return;
        }
        ReportedGlobalTextConversionDeprecation = true;
        error("DEPRECATED: Do not use the global functions for format code conversions! Use Text::StripFormatCodes, Text::StripNonColorFormatCodes, or Text::OpenplanetFormatCodes instead.");
    }
}

I am not sure if there's an easy way to get the current line number etc in angelscript (similar to what an exception would provide). However it could at least be improved to tell you which functions were deprecated. (Also it will only show it once it is called, but that's probably okay)

It might also be nice to have a mode to throw exceptions instead of warning, which would give you the line number details.

@codecat this is probably something I can work on and send you a new Compatibility.as for if that works.

ezio416 commented 4 months ago

Throwing exceptions seems like a bad idea, since as I understand it, Compatibility.as exists to provide a window of time for developers to remove deprecated things from their own plugins before they are removed entirely, which would of course throw exceptions in the future due to functions not existing.

XertroV commented 4 months ago

The exceptions would be a mode that you enable while debugging to find the spots where you call that code. Not a permanent thing.

codecat commented 4 months ago

There's a function to dump the current stack trace to log which would help here.