sampctl icon indicating copy to clipboard operation
sampctl copied to clipboard

Optional dependencies

Open Y-Less opened this issue 6 years ago • 9 comments

Is it possible to do something like this from an include:

#tryinclude <streamer>

#if defined _inc_streamer
	#define CREATE_OBJECT CreateDynamicObject
#else
	#define CREATE_OBJECT CreateObject
#endif

And have that include be somehow predicated on the explicit dependencies the end-scripter includes. So the include I'm distributing will not have the streamer plugin as a hard dependency, and even if the include exists in pawno/include it won't actually get pulled in when using sampctl unless the end-user has an explicit dependency on it. So their dependencies may look like MyAwesomeInclude, streamer, while mine are pawn-stdlib (or something like pawn-stdlib, (streamer) to indicate optionality (is that a word)).

As I said, this would be included if they were building through sampctl and depended on streamer, OR they were building through pawn and pawno/include/streamer.inc existed. The former because then sampctl controls the plugins and can guarantee that it will be loaded, the latter because there's no other option.

Y-Less avatar Jan 09 '18 15:01 Y-Less

Speaking of, it would be even nicer if there was a way (plugin?) to correctly load plugins at run-time from #pragma library constructs. Currently the only plugin that supports that AFAIK is sscanf on npcmodes, and that had to be specially written to support the different API. A wrapper would be nice, but that's totally aside from this issue.

Y-Less avatar Jan 09 '18 15:01 Y-Less

I had a think about this today while working on ScavengeSurvive/container - this library provides no UI (dialogs/textdraws) but there are:

  • a package that implements dialogs for this
  • a package that implements a textdraw-based UI

these two packages export the exact same interface (a function to display a container's contents to a player, a function to close it, etc) and I was trying to think of a way I could provide both of those to some other package while giving the user the option to choose between them or even implement their own textdraw based UI that fits their gamemode.

Similar problem that could probably be tackled with the same solution - I don't want to explicitly import either the dialog or the textdraw interfaces for my package but I still want to have the API available for testing and such. This can actually be solved already via devDependencies (sampctl package install --dev Some/Project)

So I was thinking of how Go handles conditional compilation, it provides a set of pragmas via comments (Go's version of directives - because it doesn't really have a precompiler like C) you can place // +build darwin freebsd netbsd openbsd to include that file only in compilation on those platforms.

Which could be a possible solution for this, a way for Pawn packages to communicate to the build toolchain some information about its desired state or available interface...

Another method I was thinking of actually includes an older way I was going to handle dependencies: it involved parsing Pawn source files for #include/#tryinclude directives and using those to pull in what it requires, however this would also require resolving #if-else directive logic in some way and it turned into a way bigger job than I thought...

Southclaws avatar Jan 10 '18 15:01 Southclaws

You could use forwards:

forward RequiredUIFunc1(string[]);
forward RequiredUIFunc2(colour);

Then if there is nothing that implements that you get native errors.

Y-Less avatar Jan 10 '18 15:01 Y-Less

Ooh I completely forgot about forwards! That and devDependencies probably solves most of my issues.

But I'm still interested in exploring build pragmas, I might open another issue for that.

After re-reading the original post, from what I understand this is already possible from a technical standpoint - there's just no way to declare it yet. The #tryinclude would work if any other library in the build pulls in the required library, and thus the #if defined checks would also work. There just needs to be a way to say "this library can work with and without library X".

Southclaws avatar Jan 11 '18 23:01 Southclaws

I think something like this should be added and Composer's suggested packages could be a good example to follow here.

Composer.json:

    "suggest": {
        "psr/log": "For optional PSR-3 debug logging",
        "league/oauth2-google": "Needed for Google XOAUTH2 authentication",
        "hayageek/oauth2-yahoo": "Needed for Yahoo XOAUTH2 authentication",
        "stevenmaguire/oauth2-microsoft": "Needed for Microsoft XOAUTH2 authentication",
        "ext-mbstring": "Needed to send email in multibyte encoding charset",
        "symfony/polyfill-mbstring": "To support UTF-8 if the Mbstring PHP extension is not enabled (^1.2)"
    },

Console output:

phpmailer/phpmailer suggests installing psr/log (For optional PSR-3 debug logging)
phpmailer/phpmailer suggests installing league/oauth2-google (Needed for Google XOAUTH2 authentication)
phpmailer/phpmailer suggests installing hayageek/oauth2-yahoo (Needed for Yahoo XOAUTH2 authentication)
phpmailer/phpmailer suggests installing stevenmaguire/oauth2-microsoft (Needed for Microsoft XOAUTH2 authentication)
phpmailer/phpmailer suggests installing symfony/polyfill-mbstring (To support UTF-8 if the Mbstring PHP extension is not enabled (^1.2))

// or...
facebook/graph-sdk suggests installing paragonie/random_compat (Provides a better CSPRNG option in PHP 5)
facebook/graph-sdk suggests installing guzzlehttp/guzzle (Allows for implementation of the Guzzle HTTP client)

Suggested packages either allow some additional functionality to be added to the library or increase the performance or reliability of existing code (or just make it use a polished library instead of some homebrewed code). My package samp-td-string-width has wrappers using PawnPlus strings instead of regular strings for most functions. I wouldn't want to make PawnPlus a dependency just because of this, but I'd also not like to let this extra feature go unnoticed. Another example would be samp-gui that could benefit from YSF, but I would rather not make it a dependency due to the strange incompatibility with sampctl.

Alongside this I'd also like to see a convention (or more of a guideline in the sampctl wiki) added that states that suggested dependencies can be #tryincluded without any checks for definitions like SAMPGUI_USE_YSF. Even if the user didn't install the suggested package, but it's there and available to be included, then another package is already using it anyway.

kristoisberg avatar Oct 24 '18 17:10 kristoisberg

How would you not require define checks? For some things they aren't needed, but at some point you are going to need to branch based on available other includes.

Y-Less avatar Oct 24 '18 17:10 Y-Less

That's also how Composer's suggested packages work. If they are available, they are used, otherwise they obviously aren't.

private static function detectDefaultClient()
{
    if (extension_loaded('curl')) {
        return new FacebookCurlHttpClient();
    }

    if (class_exists('GuzzleHttp\Client')) {
        return new FacebookGuzzleHttpClient();
    }

    return new FacebookStreamHttpClient();
}
if ($this->Debugoutput instanceof \Psr\Log\LoggerInterface) {
    $this->Debugoutput->debug($str);

    return;
}
// fallbacks

Dependencies should obviously be the way to go, especially if they are an integral part of the library. Both of my examples have reasons why I'd like to keep them optional. samp-td-string-width offers wrappers using PawnPlus functions and they are only included when PawnPlus is (there's a #tryinclude at the top of the file). While YSF offers a more convenient and efficient way of changing the positions of textdraws, it also has a massive drawback.

kristoisberg avatar Oct 24 '18 17:10 kristoisberg

This could be done with two methods:

  • parse the source code for #tryinclude lines and look those up somewhere
  • add a suggestions field to pawn.json that just suggests packages that improve it

Southclaws avatar Oct 27 '18 12:10 Southclaws

The second option is exactly what I suggested, there's no need to make this any more complicated than that.

kristoisberg avatar Oct 29 '18 09:10 kristoisberg