northwood-studios / NwPluginAPI

Official server-side plugin system for SCP: Secret Laboratory game.
GNU Lesser General Public License v3.0
78 stars 45 forks source link

Role and Team should be part of API #36

Open KarlOfDuty opened 1 year ago

KarlOfDuty commented 1 year ago

Something like this or just include the role and team ids from base game: https://github.com/ServerMod/Smod2/blob/master/Smod2/API/Role.cs

moddedmcplayer commented 1 year ago

I really don't see why the base-game ones aren't good enough

KarlOfDuty commented 1 year ago

Because then you have to include the server binary as a dependency. Imo everything commonly used by plugins should be part of the api.

moddedmcplayer commented 1 year ago

There is no way around including the server assembly lol

KarlOfDuty commented 1 year ago

Not currently, no.

But with Smod which is barely any larger than this api we managed to remove almost all need for the server assembly. One of the biggest plugins, admin toolbox, only had about 3 places which required the server assembly in the end.

KarlOfDuty commented 1 year ago

I can add that the reason you would want this is that the API would change a lot less than the server assembly, sometimes allowing a small plugin to be untouched for years but still work completely fine. Obviously this won't happen this early on in the API's development as it is going to change a lot in the beginning but it was very useful when things got settled down in smod.

moddedmcplayer commented 1 year ago

My man as long as we have the ReferenceHub field we will, one way or another, have to reference the server assembly. Your argument that they will make plugins last longer dosent even make sense, they would simply be duplicates to the base-game ones, no matter what.

KarlOfDuty commented 1 year ago

What do you mean? Does the API require the server assembly to build? Isn't it the other way around?

Plugins would last longer due to the API being very careful with renaming and refactoring things while the main game may change the names of objects requiring plugins to be rebuilt with the change in order to work.

This would be especially annoying when functionality is moved from one component in the game to a completely different one as the developer would then have to try to search the decompiled server assembly to try to find where it went.

moddedmcplayer commented 1 year ago

Adding wrapper that insignificant would just exponentially increase the amount of code needed 99% of which is just going to be double. If there would be a team added this enum would get updated, too. It literally makes no difference. And if you design your to treat enums like numbers, which they are, then it wont make a difference. A dict with numbers as keys wont care if there are suddently numbers bigger than 18, and a logger using .ToString() wont care if an extra string is added. There is no way around referencing the server assembly, and adding Role and Team would simply be duplicates.

KarlOfDuty commented 1 year ago

You keep talking like it's some theoretical impossible concept but this is how almost all smaller smod plugins worked. Even the largest ones were almost rid of the server assembly at the previous update.

The job of an API is to provide an interface of an external project to interact with yours. In order for interacting projects to be most compatible the API should change as little as possible outwardly even if the backend changes. It is the role of the API to abstract these functions, not just give access to an ever-changing codebase and telling them to keep up. If people have to go around the API to interact directly with your binary for very basic things then the API is a bit incomplete.

I don't really get your point with enum names, just using integers instead would make for very unsafe code if the enum order changes.

And it also doesn't address changing names of functions, variables, namespaces, classes.

An even more annoying issue though would be when things get refactored in the game and the function I was using is now in a completely different object in a new subclass. Or someone has just moved a class into a new directory, changing it's namespace. This would be much more difficult for a plugin developer to follow as they do not have access to the game source code.

All of this requires detective work which shouldn't even require a change as the plugin will still do the exact same thing as before.

It is quite literally the definition of an API to have abstract funtions where the backend code is irrelevant to the user. This allows the backend developers to change their code freely without impacting API users as they just redirect the API function to use the newly refactored code instead.

Summary: Basically a major game update with the current system requires every plugin developer to individually adapt their code to the often very refactored game without actually having access to the source code.

I argue that the API should cover all the most common functions and the game developer renaming a function that gets called from 3 places in the game would just have to rename it in 4 places instead.

Having one knowledgeable person do the job once instead of all plugin developers doing the job individually with no knowledge of how or why the game changed internally.

moddedmcplayer commented 1 year ago

Role enum would still be a duplicate of the base-game one.

KarlOfDuty commented 1 year ago

To begin with yes, but if a role changes name as has happened many times in the past the API version would include backwards compatibility to the old one.

For instance when the mtf guard changed name to mtf cadet the enums would look like this:

Game version:

enum RoleID
{
    ...
    MTF_CADET = 15,
    ...
}

API version:

enum RoleID
{
    ...
    MTF_CADET = 15,
    MTF_GUARD = 15,  // Deprecated
    ...
}

(Although in this specific example I think I would gather all the deprecated ones in its own section at the bottom so you could easily check that the top part is identical to the game but you get the point)

And at some point when the API has a large update or you simply just feel like you have a lot of deprecated functions you declare the next API version a major version with breaking changes and remove all or most of the deprecated parts.

KarlOfDuty commented 1 year ago

Here is an example with the current items on top and the deprecated names at the bottom of the enum: https://github.com/ServerMod/Smod2/blob/179b2c694d027a3b9dbfd1bc207506244d98b1b7/Smod2/API/Item.cs

moddedmcplayer commented 1 year ago

wtf hell no

moddedmcplayer commented 1 year ago

When update happens and new items added you probably want your plugins to differentiate between new and old and not having a mix between the 2.

KarlOfDuty commented 1 year ago

For what reason? You want to break all plugins using keycards just because they are now called \<name>_KEYCARD instead of KEYCARD_\<name>? How does this help anyone?

APIs are supposed to be as stable as possible, you can't do that without backwards compatibility between game updates.

moddedmcplayer commented 1 year ago

Imagine making a plugin and your IDE shows 3 different items with the same name id be hella confused.

KarlOfDuty commented 1 year ago

I'd say hmm thats a bit strange, ctrl-click on it and see it says deprecated on two of them.