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

Add basic support for TerraFirmaCraft #115

Merged
merged 6 commits into from Apr 19, 2015
Merged

Add basic support for TerraFirmaCraft #115

merged 6 commits into from Apr 19, 2015

Conversation

3TUSK
Copy link
Contributor

@3TUSK 3TUSK commented Apr 17, 2015

I did a rough test, and the function seems work.
(New recipe can be shown in NEI correctly, with help of TFC-NEI Plugin)
Not sure if there is bug which hasn't been found.
If there is someone who can help me test, I will say appreciate. @Yulife

@Yulife
Copy link
Collaborator

Yulife commented Apr 17, 2015

That's quite impressive work, thanks a ton! Will test when I get home =) Does Oredict, Fuzzy Metadata and NBT-Data work with it?

@Yulife
Copy link
Collaborator

Yulife commented Apr 17, 2015

Also, is this everything doable with the TFC API? Iirc there were ovens and fireplaces as well. :)

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 17, 2015

I haven't test fuzzy, nbt and oredict yet, but seems TFCraft API is using vanilla ItemStack. Not sure if oredict can be supported.
As for TFCraft forge and crucible, it seems impossible to add support. Currently I am thinking about adding support for knapping stone, clay and leather.

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 17, 2015

Another thing, TFC API has a LoomManager and PlanManager, but loom requires a ResourceLocation, and PlanManager (which is for plan in an anvil) requires IIcon. (Which means that there must be already proper texture file somewhere.)
I am not sure whether they are possible, but if possible I will start to work on them.

Note: double checked Thaumcraft and Botania support, it is doable to use IIcon and ResourceLocation. Add to my todo list.

@Yulife
Copy link
Collaborator

Yulife commented Apr 17, 2015

That's unfortunate - and the campfire?

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 17, 2015

Campfire has no related API file. I will double check whether there is possibility to add support after school.
Btw, it should be doable to add loom recipe and new anvil plan. I will work on that after school.
Also, I found #43

@Yulife
Copy link
Collaborator

Yulife commented Apr 17, 2015

The campfire has no API integration...? Huh, I wonder how they do it, can you clear this up @Kittychanley? :)

Thanks in advance ~

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 17, 2015

Double checked again, seems I just ignore something... HeatIndex
Also added into my todo list (after schoool)

@Yulife
Copy link
Collaborator

Yulife commented Apr 17, 2015

Oops, lol! Thanks ;)

@3TUSK 3TUSK changed the title Add support for quern, kiln, barrel/vessel and anvil from TerraFirmaCraft Add support for quern, loom, kiln, firepit, barrel/vessel and anvil from TerraFirmaCraft Apr 18, 2015
@3TUSK 3TUSK changed the title Add support for quern, loom, kiln, firepit, barrel/vessel and anvil from TerraFirmaCraft Add support for quern, loom, kiln, forge/firepit, barrel/vessel and anvil from TerraFirmaCraft Apr 18, 2015
@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 18, 2015

So far:
I can make sure that quern, anvil, fogre/firepit are available.

Kiln is in a special situation, since a item which can be put into kiln must be a tfc ceramic thing.
(due to player.inventory.getCurrentItem().getItem() instanceof ItemPotteryBase)
As a result, new kiln recipe can be recognized and shown in NEI with a tfc-neiplugin, but it actually won't work.

Loom haven't tested, since it requires a texture, and I'm bad at drawing :(
Test with missing texture, and it works!
2015-04-18_14 48 23

Barrel/Vessel are weird, new recipe can be shown in NEI (with TFC-NEIPlugin), but actually it doesn't work. I'll handle it.
Barrel and vessel now work, which can also support fluid from other mod (test with IC2exp liquid UU-Matter) and there is some note in code (I mean javadoc).
2015-04-18_14 49 52

@Yulife looks good?

@Yulife
Copy link
Collaborator

Yulife commented Apr 18, 2015

Tell me what you need, I'll draw you one :)

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 18, 2015

Sorry, I found that I can just test custom loom recipe with the "missing texture" :)
Thank you for your support!

@joshiejack
Copy link

Did you try pullrequesting this to tfc/asking them if they would accept? (Since it's always better to be in the mod if possible)?

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 18, 2015

Initial finished.
Due to the TFC API (also my skill is also limited :( ), ore-dict and fuzzy won't work well. NBT is supported, also many recipes in TFC are sensitive to NBT data.

Currently support:
Anvil, Barrel/Vessel, Firepit/Forge, Quern
Currently support (not fully):
Loom (sometimes missing texture, but it does work, so just ignore the ugly black&purple square xD);
Kiln (recipe can be registered, but it requires that inputItem instanceof ItemPotteryBase)

@joshiejack
According to this, they might hope the support is part of ModTweaker2.

@joshiejack
Copy link

Ahh ok, that sucks then. Since as with all the other mods, it's another that might break when the mod updates, and then people come crying here xD. The less the better!

@3TUSK 3TUSK changed the title Add support for quern, loom, kiln, forge/firepit, barrel/vessel and anvil from TerraFirmaCraft Add basic support for TerraFirmaCraft Apr 18, 2015
@Yulife
Copy link
Collaborator

Yulife commented Apr 18, 2015

What's the deal with "inputItem instanceof ItemPotteryBase"? Is it something special, does it need something special? Does it mean the items to it are limited? I'm confused - and by the way thanks for the hard work!

@jaredlll08
Copy link
Owner

It means that the return item must implement or extend that class.

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 18, 2015

Which means it is a little complex work for a modpack creator.
(They might need to create a new mod to add item(s) so that they can put what they want into a pottery kiln.)

@Yulife
Copy link
Collaborator

Yulife commented Apr 18, 2015

Oh. Couldn't this be done in a TFC helper in Modtweaker itself?

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 18, 2015

@Yulife I don't think so...

@Yulife
Copy link
Collaborator

Yulife commented Apr 18, 2015

@jaredlll08 is doing a lot of helpers, you think it will work?

@Yulife
Copy link
Collaborator

Yulife commented Apr 18, 2015

@3TUSK Want me to merge?

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 18, 2015

@Yulife If there is no known bug... XD

@jaredlll08
Copy link
Owner

If you can confirm no bugs. i will merge and release the mod

On Saturday, April 18, 2015, Urey.X. notifications@github.com wrote:

@Yulife https://github.com/Yulife If there is no known bug... XD


Reply to this email directly or view it on GitHub
#115 (comment)
.

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 18, 2015

Give me sometime...

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 18, 2015

Double checked, and I believe there is no bugs.

Yulife added a commit that referenced this pull request Apr 19, 2015
Add basic support for TerraFirmaCraft
@Yulife Yulife merged commit c757841 into jaredlll08:master Apr 19, 2015
@Yulife
Copy link
Collaborator

Yulife commented Apr 19, 2015

yay2
Thank you!! ~~ 👍 👍 👍

@Yulife
Copy link
Collaborator

Yulife commented Apr 19, 2015

Alright going through the handlers right now... what I noticed:

1: You can use null two in the anvil and it works. This should at least throw an error if you use null in the script two times.

2: As you can see in the picture I used "iamnotaplan" for the plan string. This does not throw an error which is worrisom. It should throw an error if the string is not precisely correct.

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 19, 2015

Y'all are very welcome XD

@Yulife
Copy link
Collaborator

Yulife commented Apr 19, 2015

And what's "int value" in the anvil handler? I can figure out all different things but "value" leaves me confused.

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 19, 2015

Ahhhh, anvil is a thing...
1.is there anyone who really add a anvil recipe with two null input? But I will consider to throw error under such a situation.
2.As for plan, there are some existed plan inside of the TFC source code...

@jaredlll08
Copy link
Owner

Just an integer, a field with the value if an int

On Sunday, April 19, 2015, Yulife notifications@github.com wrote:

And what's "int value" in the anvil handler? I can figure out all
different things but "value" leaves me confused.


Reply to this email directly or view it on GitHub
#115 (comment)
.

@Yulife
Copy link
Collaborator

Yulife commented Apr 19, 2015

Ahhh, thanks @3TUSK!
1: Yeah, it would make sense. No one would enter two inputs.
2: Yes, I know, I am going to list them on the documentation. However, if someone makes a typo, it would be neat if an error would be thrown. For example, the string "oillamp" in TFC. If I were to be in a hurry and write "oilamp" it would be very helpful if an error would be thrown instead of scavenging through the code for hours to find out why it doesn't work.

@jaredlll08 that doesn't clear anything up. I mean what it actually has in relation to TFC, for example "int req" is the Tier of anvil to be used for the recipe. What does value do?

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 19, 2015

If my memory is accurate, the int-type parameter "value" means the "crafting progress".
Click it to learn more. (check Anvil GUI 8.)
Which means a mod-pack creator need to be very very very careful to avoid a uncraftable thing

@jaredlll08
Copy link
Owner

uh.. I just tried opening my dev environment... it crashes with TFC...

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 19, 2015

what... I am using TFC 0.79.17, Forge 10.13.2.1291

@jaredlll08
Copy link
Owner

Exactly the same stuff here... foes TFC need any other mods?

@Yulife
Copy link
Collaborator

Yulife commented Apr 19, 2015

Huh, interesting. However, I am not sure what number to put in there, lol. Confused :s

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 19, 2015

TFC doesn't require a dependency... Also Iam using NEI 1.0.4.83, waila 1.5.8a
Other mods: buildcraft 7.0.1, ic2exp 2.2.717, fastcraft-1.19, minetweaker 3.0.9C
looks weird...

@jaredlll08
Copy link
Owner

well I just tried loading it up with all other modtweaker mods
and it is giving this error:
http://www.blamejared.com/GetFluxed/content/2015-04-19_17-52-37.txt

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 19, 2015

It looks like TFC's fault.
try download from http://terrafirmacraft.com/download.html ?
TFC has a coremod...
I just built a new ModTweaker and tested it as if I am a regular player...

@3TUSK
Copy link
Contributor Author

3TUSK commented Apr 19, 2015

@jaredlll08
Copy link
Owner

probably...

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

Successfully merging this pull request may close these issues.

None yet

4 participants