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

Adding OC Fluid Controller support #4

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

Adding OC Fluid Controller support #4

wants to merge 1 commit into from

Conversation

Hasunemiku2015
Copy link

Trying to add support for OC Fluid Controller, will do the FluidStack thing later.

Check if the current code has any problems.

@Exeteres
Copy link
Owner

Hi, thanks for the PR!

According to the documentation the valid signature for the return value should be like this:

image

getTankCapacity(side: number): LuaMultiReturn<[number?, string?]>;

The getInventorySize method you may have used as a hint is also incorrect. I will fix it later.

The optional parameters of the function should be marked like this:

drain(amount?: number): boolean;

Otherwise they are considered as required.

It seems that I didn't notice these problems in one of the latest PRs 🤔

I don't see any more problems here except one...
Please install the Prettier plugin and use it to format the code.
I can do it myself after accepting the pull request, but I will be very grateful if I don't have to.

@Hasunemiku2015
Copy link
Author

Okie! I am new to typescript so sorry for my poor code...

Also, would you mind me adding AE2 and Mekanism Reactor Support to this thing? (Since I need to use it)

@Exeteres
Copy link
Owner

I don't mind. Feel free to add any mod typings you need!
But create new packages for them.
In the near future I will reorganize some code and add the ability to automatically publish packages.

@Exeteres
Copy link
Owner

Note that I have extracted component APIs to the separate package, so you need to back-merge the master branch and move your declarations to the common package.
And now we also have AE2 typings thanks to @timoheijne.

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.

2 participants