Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide an API for registering custom APIs #491

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

@SquidDev
Copy link
Contributor

@SquidDev SquidDev commented Nov 18, 2017

ILuaAPI has been moved to dan200.computercraft.api.lua. One creates a new API by registering an instance of ILuaAPIFactory. This takes an instance of IComputerSystem and returns such an API.

IComputerSystem is an extension of IComputerAccess, with methods to access additional information about the the computer, such as its label and filesystem.

This closes #346, which also explains some of the rational behind this functionality.

@@ -644,6 +646,14 @@ public static void registerMediaProvider( IMediaProvider provider )
}
}

public static void registerAPIFactory( ILuaAPIFactory provider )
Copy link
Contributor

@Lignum Lignum Nov 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this should return a boolean specifying whether the API was actually registered?

Copy link
Contributor Author

@SquidDev SquidDev Nov 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only really doing it this way because that's what all the other methods do. The only way it could thrown an exception is if the reflection fails, which shouldn't ever happen anyway.

Copy link
Contributor

@Lignum Lignum Nov 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was referring to if( provider != null && !apiFactories.contains( provider ) ) failing.

@@ -244,6 +245,7 @@
private static List<IMediaProvider> mediaProviders = new ArrayList<>();
private static List<ITurtlePermissionProvider> permissionProviders = new ArrayList<>();
private static final Map<String, IPocketUpgrade> pocketUpgrades = new HashMap<>();
private static final List<ILuaAPIFactory> apiFactories = new ArrayList<>();
Copy link
Contributor

@dmarcuse dmarcuse Nov 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By using a Set you can avoid explicitly checking if it's been registered later

Copy link
Contributor Author

@SquidDev SquidDev Nov 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an argument that this allows for some level of priority ordering. I'm not entirely sure whether such an ordering is needed though - ideally mods shouldn't be overwriting each other's APIs, but it's something worth discussing.

Copy link
Contributor

@dmarcuse dmarcuse Nov 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's any particular reason order is important, but if it is, LinkedHashSet maintains order

ILuaAPI has been moved to dan200.computercraft.api.lua. One creates
a new API by registering an instance of ILuaAPIFactory. This takes an
instance of IComputerSystem and returns such an API.

IComputerSystem is an extension of IComputerAccess, with methods to
access additional information about the the computer, such as its label
and filesystem.
@SquidDev SquidDev force-pushed the feature/api-api branch from a30b207 to 5584746 Nov 19, 2017
@thatcraniumguy
Copy link

@thatcraniumguy thatcraniumguy commented May 23, 2018

Can one of the admins verify this patch?

ccserver pushed a commit to ccserver/ComputerCraft that referenced this issue Sep 16, 2019
…api-api

Provide an API for registering custom APIs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants