CitiesSkylinesMods / TMPE

Cities: Skylines Traffic Manager: President Edition
https://steamcommunity.com/sharedfiles/filedetails/?id=1637663252
MIT License
571 stars 85 forks source link

Complete TMPE API #685

Open kianzarrin opened 4 years ago

kianzarrin commented 4 years ago

from #678

@krzychu124 Also we should update API project interfaces and make strong version instead of [1.0.*] Probably then we'll need to log changes in separate section of Changelog , maybe even export whole project as a new repository and release each version separately (it still could be linked as submodule in main repo). Any mod dev could download API lib and import it in his mod source code and relay on provided interfaces when he/she want to make changes to TM:PE settings. Still thinking how to expose manager instances reliably (return interface instance instead of using it directly) :/

@aubergine10 If someone can provide brain dump of basics of using the API I'll tidy it in to markdown and we can add some docs to the relevant folder and maybe link from contributing guide or something. I assume it just spits out a DLL that devs can reference from their projects to interface to TMPE at runtime?

@krzychu124 The missing part is: reliable source of instances, since Managers in TM:PE are static. Probably we would need class in API which would act as container to provide instances or we could use some IoC container like Unity Quick example of usage:

public class InstanceProvider {
public ITrafficPriorityManager GetPriorityManager() {
return TrafficPriorityManager.instance;
}
}

then other mods could use it as follows (it's only example):

bool hasNode = InstanceProvider.GetPriorityManager().HasNodePrioritySign(1234);
//TrafficPriorityManager has declaration of bool HasNodePrioritySign(ushort nodeId);

I think it's pretty standard Plug-in architecture. Having instance provider in API you get ability to modify everything in main project without breaking other mods. You can change everything, class/property names, types, the only thing that should stay intact is API call. It should always return instance compatible with interface. In my opinion strong versioning is important for maintaining compatibility. You can add more declarations but you shouldn't change existing, since it will cause breaking change. Say mod x use API v1 and wants to call some method, it shouldn't break anything when we update API to v2 with new method declarations unless we remove/change any old declarations. The only thing which might cause issues I can imagine is hot-reload, but we would need to test out it. I suggest reading about extern keyword, as some way to resolve runtime versioning issues :) I'm not C# dev so I might be completely wrong with this approach. I'm open on other ideas.

kvakvs commented 4 years ago

Consult some C# dev who knows the best practices on this. I can give you a dozen of best practice ideas how to create an API but none of them are C#.

originalfoo commented 4 years ago

@dymanoid is usually an oracle or has good links on all things c#

dymanoid commented 4 years ago

For public API, the best practice is to use Semantic Versioning - the version format is major.minor.patch. You modify the major version when the public API gets breaking changes, i.e. compatibility. You modify the minor version to inform that there are new features, but the public API wasn't changed. You modify the patch version for bugfixes: neither new features, nor API changes. This is independent of the programming language in use.

InstanceProvider.GetXXX() is a static factory and thus a kind of ServiceLocator (other name: ServiceProvider) anti-pattern (with slightly increased type safety). I wouldn't suggest using that.

Unfortunately, there is no neat way to use IoC in Unity-based games. Neither has Cities:Skylines any supporting features for that.

I would go with a single static factory that returns an interface-based facade for your API. Something like:

public static class TrafficManager
{
    public static ITrafficManagerAccess GetAccess() { /* ... */ }
}

Then, you can organize your ITrafficManagerAccess as you wish without forcing the clients to bind to any static members of the API.

krzychu124 commented 4 years ago

I assume that single static factory should be a part of main project (outside of public API) returning entirely new interface instance which would wrap other manager interfaces, right? Am I understand it right(based on our current API interfaces)? E.g.:

public interface ITrafficManagerAccess
{
    ILaneArrowManager LaneArrowsManager(); //or `GetLaneArrowsManager`
    ILaneConnectorManager LaneConnectorManager();
    //...
}

Then we could safely change visibility of our manager interface implementations to internal (forcing access through that facade and public API)

kvakvs commented 4 years ago

Create a new Github project, a C# library call it TMPE API It will do this for known supported TMPE versions, and provide the serialization of call parameters, and deserialization of response (also known as "API access") to other mods who decide to use it.,

originalfoo commented 4 years ago

673 would be useful here.