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

Cortana added to robots.name #1003

Closed
wants to merge 7 commits into from
Closed

Cortana added to robots.name #1003

wants to merge 7 commits into from

Conversation

KJA1582
Copy link
Contributor

@KJA1582 KJA1582 commented Apr 2, 2015

Now also includes mod integration drivers for Flaxbeard's Steam Power.

NOTE: You might have to look into the dependencies, I couldn't figure it out. This why there is a Flaxbeard.jar (it is version 0.28.7, latest at the time of writing) in the main folder. I used the compile dependency in build.gradle.

KJA1582 added 5 commits April 2, 2015 12:08

Verified

This commit was signed with the committer’s verified signature.
W-A-James Warren James

Verified

This commit was signed with the committer’s verified signature.
W-A-James Warren James

Verified

This commit was signed with the committer’s verified signature.
W-A-James Warren James
…C1.7.10

Verified

This commit was signed with the committer’s verified signature.
W-A-James Warren James

Verified

This commit was signed with the committer’s verified signature.
W-A-James Warren James
…C1.7.10
@@ -178,6 +178,9 @@ dependencies {

compile 'com.google.code.findbugs:jsr305:1.3.9' // Annotations used by google libs.

//EDITS for Flaxbeard's Steam Power
compile files('Flaxbeard.jar')
Copy link
Member

Choose a reason for hiding this comment

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

Please add the dependency in a way it can be autodownloaded. If FSP has a maven, that'd be ideal, otherwise add it the the CoFH stuff is referenced, e.g. (i.e. via Curseforge).

@fnuecke
Copy link
Member

fnuecke commented Apr 4, 2015

Aside from the above, the drivers look OK! When done, please squash your commits.

@fnuecke
Copy link
Member

fnuecke commented Apr 4, 2015

Aaand I just read your not. Derp. As I said above, look into how the CoFH ones are done. You get the numbers and by looking at the download URL when you get the file from Curseforge.

@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

What is squashing my commits? I don't use git that offen/much

Verified

This commit was signed with the committer’s verified signature.
W-A-James Warren James
…C1.7.10
@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

By looking at the CoFH, you do mean the ones in build.gradle? Cause I don't quit understand how they are defined as such.

@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

Ok. Got the autodownloady bit figured out, but now it fails on the javadoc because fsp is called Flaxbeard'sSteamPower and it doesn't like the ' in it. How can I exclude it?

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
W-A-James Warren James
@Vexatos
Copy link
Contributor

Vexatos commented Apr 4, 2015

I hope you know that Computronics already has support for all of these so it's actually pretty unnecessary...

Sorry, something went wrong.

@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

CC has, but unless CC is installed with OC, you can't use it.

Sorry, something went wrong.

@Vexatos
Copy link
Contributor

Vexatos commented Apr 4, 2015

Of course you can, Computronics adds FSP integration to ComputerCraft and OpenComputers.

@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

I couldn't access it while only having OC installed. in RR3, i did have access to all CC methods.

@Vexatos
Copy link
Contributor

Vexatos commented Apr 4, 2015

Well, is definitely is there, and it also definitely is being registered.

@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

Nice, but it isn't OC native.

@Vexatos
Copy link
Contributor

Vexatos commented Apr 4, 2015

...

Isn't that what compat addons are for? Adding integration with other mods?

@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

Native is like BootCamp. Computronics is more like Parallel Desktops. Nothing wrong with that, but thats the way I see it.

@LizzyTrickster
Copy link
Contributor

Also having loads of mod Compatibility in OC itself puts more strain on Sangar because he has to check if any of their APIs have breaking change and potentially support more than one version of the API in some rare cases..

Just my two pennies

**Edit spelling -.-

@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

Thats a good point.

@vifino
Copy link
Contributor

vifino commented Apr 4, 2015

I agree with @LizzyTrickster, and I think using compat addons is just fine.

@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

There is nothing wrong with compat add ons, don't get me wrong. I just wanted a more native implementation, thats all. After all, thats why open software exists, to build on it and expand it.

@fnuecke
Copy link
Member

fnuecke commented Apr 4, 2015

Muh inbax.

So here's the thing: Computronics is, AFAIK, in every modpack that has OC in it anyway (for good reason), and soon OpenPeripheral, too, will have OC support. So there's that. I understand your desire to have the integration bundled with OC itself, as it'd make it more "batteries included", but as @LizzyTrickster pointed out, in the long run maintaining the integration is something I will have to do, either because whoever added it isn't available anymore, or has no time, or it has to get fixed quickly. And it piles up.

That doesn't mean I want no integration code in OC, on the contrary. But adding new integration only makes sense to me, if it's not already present in one (or possibly two!) other mods, each of which are quite popular. And seeing as this is already present in Computronics, I don't think it makes much sense to duplicate the driver. And it doesn't make much sense to move that code from Computronics to OC, either, as it supports other mods, too, which is a huge argument in favor of having separate integration mods.

Thanks a lot for your work and interest nonetheless! If you find any other things that might make sense to add integration for - ideally without requiring JAR dependencies, but that's admittedly rare - please do suggest and discuss that!

@fnuecke fnuecke closed this Apr 4, 2015
@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

Will do, I actually didn't know there already was integration for this, so this one is my mistake.

Anyway, I still have that failing javadoc on the build, how do I solve it? Using the exclude statement didn't do anything, I probably set it up wrong somehow.

@LizzyTrickster
Copy link
Contributor

@Vexatos did point out the intergration....

@fnuecke
Copy link
Member

fnuecke commented Apr 4, 2015

No idea what that error is you're getting there, sorry.

He did, but only after the PR was made, unless I missed something?

@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

AFTERWARDS, I didn't know this mod even exists/did it.

@KJA1582
Copy link
Contributor Author

KJA1582 commented Apr 4, 2015

It can't find the packages sSteamPower, which occurs since in the name there is an ', which also is the string tag

@MightyPirates MightyPirates locked and limited conversation to collaborators Apr 4, 2015
@fnuecke
Copy link
Member

fnuecke commented Apr 4, 2015

No clue, but that's more of a Gradle question, and there are places for that. I'll be locking this before it gets any more off topic...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants