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

Issue 103 External API #1667

Merged
merged 6 commits into from
Jun 9, 2016
Merged

Conversation

hvacengi
Copy link
Member

@hvacengi hvacengi commented Jun 7, 2016

Related to #103

This provides an avenue for other KSP mods to provide addon support. The biggest "feature" is that kOS now has the ability to walk all of the loaded assemblies, and create a list of types to register with "manager" classes. The existing Binding and Function managers have been ported to use this system as well.

Essentially manger classes define an attribute, inherited class, or interface that identifies classes that they will manage. Then the AssemblyWalkAttribute will run through every type that's currently loaded (but not in the GAC) and see if it meets any of the multiple managers' criteria. If it does, it registers the type with that manager.

The AddonManager will register classes decorated with the kOSAddon attribute and which inherit from Suffixed.Addon.

Of note, the 3rd party assembly must include the following line inside of AssemblyInfo.cs to ensure the proper loading order of the assemblies:

[assembly: KSPAssemblyDependency("kOS", 0, 20)]

Features

  • Walk assemblies to find classes by attribute, inheritance, or interface
  • Manager classes register each type for use during main runtime.
  • AddonManager allows Addon's to be loaded from other assemblies
  • Error messages in the log if a class is decorated by does not implement a feature as expected (nag messages)
  • Update kOS nomenclature attributes to work with the additional assemblies (adapt to use AssembliyWalkAttribute)
  • Sample 3rd party addon to use as an example
  • Documentation for 3rd parties to use

AssemblyWalkAttribute.cs
* New class to manage walking the loaded assemblies looking for classes, interfaces, or attributes
* Decorate manager classes with this attribute to define how the class should register other types

Bootstrapper.cs
* Add call to AssemblyWalkAttribute.Walk from the Start method.

BoundVariable.cs
* Clear the cache if you set the bound variable

Opcode.cs
* Switch casting method to avoid casting error.

SafeFunctionBase.cs, FunctionBase.cs, FunctionManager.cs,
SafeBindingBase.cs, Binding.cs, BindingManager.cs, kOSProcessor.cs
* Create new classes in kOS.Safe to serve as safe base classes.
* Move as much logic into kOS.Safe as possible.

kOS.Safe.csproj, kOS.csproj
* Reference new classes and remove old references.
Convert existing Addons to the new attribute system

AddonManager.cs
* New manager class for all addons.
* Receives the register calls of AssemblyWalkAttribute
* Contains a static list of all kOSAddonAttributes and associated types
* Instantiated by the processor as a member of kOS.SharedObjects

kOSAddonAttribute.cs
* New attribute class for decorating kOS Addon classes.
* Important to remember that the class must inherit from kOS.Suffixed.Addon as well (defined by AddonManager)

kOS.csproj
* Add references to new classes.

AddonList.cs
* Rework AddonList to create suffixes based on Addons in AddonManager
* Add new suffixes for HASADDON and AVAILABLE to allow scripts to check if the addon is installed and if it is available

kOSProcessor.cs, SharedObjects.cs
* Add references to the new AddonManager
TelnetMainServer.cs
* use `GetValue` instead of the string index to return permission
setting values in order to stop exceptions when loading and the key is
not present.
@hvacengi hvacengi added documentation Change the Sphinx user documents. Not Ready Author is warning Reveiwer not to merge this PR yet. More edits are expected. labels Jun 7, 2016
@hvacengi hvacengi added this to the v1.0.0 milestone Jun 7, 2016
AssemblyWalkAttribute.cs
* Fix ToString to be more useful by identifying its class, and then the
criteria.
* Add a could comments
* Fix attempting to force Interface/Inherited conditions to accept an
attribute condition as well.
* Add more detail to error messages.
* Remove logging output for every class that does not satisfy
Interface/Inheritance conditions.

KOSNomenclature.cs
* Implement AssemblyWalkAttribute.
* Move logic from assembly based methods into stand alone single type
methods.

Bootstrapper.cs
* Remove call to KOSNomenclature PopulateMapping
Addon Readme.md
* Add some documentation for how to implement a new Addon.
@hvacengi hvacengi removed the Not Ready Author is warning Reveiwer not to merge this PR yet. More edits are expected. label Jun 8, 2016
on your central Addon class.
* All new structures, including your Addon, must be decorated with a
`[KOSNomenclature]` attribute, identifying it's "kOS Name" (the value returned
by the `typename` suffix).
Copy link
Member

Choose a reason for hiding this comment

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

The document should probably mention the parameters to KOSNomenclature and not make it look like it's a bare paremeter-less attribute. (i.e. "must be decorated with a [KOSNomenclature("put the kos name here")] attribute.....")

Copy link
Member Author

Choose a reason for hiding this comment

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

Absolutely correct. If for no other reason than consistency since that's what I did with kOSAddon above.

@Dunbaratu
Copy link
Member

Dunbaratu commented Jun 8, 2016

Should issue #103 get auto-closed when this gets merged? It only sort of resolves half of what 103 was talking about. (The half mentioned later in comments that wasn't even part of the original issue - that of having kOS scripts invoke addon code.) The original issue description talks about going the other way around - having addons invoke kOS scripts. Not that I think that's a very good idea to implement, but it is what issue #103 was asking for. I think I'd rather close issue #103 as a "wont fix" rather than claiming it got fixed by this PR, since this PR doesn't quite really do what 103 was asking for, and is instead doing something else orthogonal to 103.

@Dunbaratu Dunbaratu added enhancement Something new (not a bug fix) is being requested refactor An infrastructure change that shouldn't change end result. labels Jun 8, 2016
@Dunbaratu
Copy link
Member

I'd be ready to merge this now, depending on that last question about issue 103, and the fact that we don't have a good example case of using it yet (how could we - it has to get merged to develop before other people start using it). Code-wise it looks fine.

@hvacengi
Copy link
Member Author

hvacengi commented Jun 9, 2016

Hm, I thought I posted a comment to this last night, but apparently didn't. A re-read of #103 indeed makes it look like this is less related to that precise issue. Telnet really is a closer solution to that problem. Though using this new system, a mod will have access to the cpu the same way that the processor and the terminal do, so it would be possible for them to execute arbitrary code on the processor in much the same way. When this and your run PR are both merged, we should look at whether or not there is a direct public route that the addon would be able to call a method like shared.CPU.RunCode

Addon Readme.md
* Clarify assembly/project separation.
* Reference the parameter of the KOSNomenclature attribute

AssemblyWalkAttribute.cs
* Log assembly location when it cannot be loaded
* Create a couple of constant string format fields.
* Remove uneeded zero parameter check in CheckMethodParameters

Binding.cs
* Remove Update method that was hiding the inherited Update method.
@Dunbaratu Dunbaratu merged commit c611b04 into KSP-KOS:develop Jun 9, 2016
@hvacengi hvacengi deleted the issue-103-external_api branch June 23, 2016 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Change the Sphinx user documents. enhancement Something new (not a bug fix) is being requested refactor An infrastructure change that shouldn't change end result.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants