SetFailState - option to suppress stack trace

[Alienmario]

I think that when a clear reason is passed to SetFailState, there should not be any need to print the stack trace. It's certainly not "friendly" to end users. This could be added as an optional parameter.

In addition, the wording makes it seem like the plugin is to blame, rather than something with the user's environment, which is most likely the cause. Another issue is it prints the absolute path of the source file, from the person who compiled the plugin. But that is probably unrelated to this.

Current:

L 01/16/2022 - 21:08:32: [SM] Exception reported: This engine is not compatible (...)
L 01/16/2022 - 21:08:32: [SM] Blaming: srccoop.smx
L 01/16/2022 - 21:08:32: [SM] Call stack trace:
L 01/16/2022 - 21:08:32: [SM]   [0] SetFailState
L 01/16/2022 - 21:08:32: [SM]   [1] Line 28, D:\Games\SteamLibrary\steamapps\common\Black Mesa Dedicated Server\bms\addons\sourcemod\scripting\SourceCoop\scripting\srccoop.sp::LoadGameData
L 01/16/2022 - 21:08:32: [SM]   [2] Line 111, D:\Games\SteamLibrary\steamapps\common\Black Mesa Dedicated Server\bms\addons\sourcemod\scripting\SourceCoop\scripting\srccoop.sp::OnPluginStart

Suggestion - just to illustrate:

L 01/16/2022 - 21:08:32: [SM] Exception reported: This engine is not compatible (...)
L 01/16/2022 - 21:08:32: [SM] Pausing plugin: srccoop.smx

[dvander]

Does AskPluginLoad cover this case? You should be able to check engine stuff there.

[Alienmario]

This would cover the plugin load check but I have other cases per map load. I can accomplish this with unloading the plugin with server command but prefer SetFailState, because it feels cleaner.

[dvander]

A changelevel to certain maps will permanently disable your plugin?

[Alienmario]

Well, it might be unusual, but for example here: https://github.com/ampreeT/SourceCoop/blob/9a1e28af22cc9c9a583a664894d696800b711805/scripting/srccoop.sp#L164

The first SetFailState is for listenservers only and from testing, failed plugins reload in this case, but there had to have been a disconnect or map command in between. The second SetFailState is also for dedicated, and yes that should kill the plugin.

[Kenzzer]

sm plugins load/unload/reload aren't meant to be used by server operators, or automatons. They merely dev tools, for plugin debugging. If your plugin mustn't work on some specific map, then you should look into how to make it quarantine itself. (If I understood your issue properly, I might not have, so disregard this part of my message if I have).

SetFailState must be used when your plugin enters an error state it cannot recover from. That said, I too would like a cleaner error message output. As the lines/stack trace, where SetFailState occurred is most of the time irrelevant, and only the message part is.

[dvander]

The second fail state looks like it'd trigger on newer SourceMod versions, so us adding a new flag won't help you much :)

I don't like it when programs fail by dumping a huge stack trace, it's bad UX. So I do like the idea of having a less-verbose way to do this. Let's make sure the use case is properly met first though, and I'm not seeing it here exactly.

[Alienmario]

Alright, the last use cases I can think of are usually done in OnPluginStart. Should these actually be moved to AskPluginLoad2?

• Loading gamedata

GameData pGameConfig = LoadGameConfigFile(GAMEDATA_NAME);
if (pGameConfig == null)
    SetFailState("Couldn't load game config %s", GAMEDATA_NAME);

• Failure when setting up hooks and SDKCalls from such gamedata
• Failure to read some other required configuration file, which could be non existent or in wrong format.

[dvander]

Anything not dependent on extensions or other plugins (I think) can be moved to AskPluginLoad. If you still need calls in OnPluginStart after satisfying that, a new native might be reasonable.