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

Refactor machine tool actions #3275

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

RecursivePineapple
Copy link
Contributor

This PR refactors tool interactions with machines and removes the deprecated onXRightClicked methods. The actions for single block machines were cleaned & polished up but untouched functionality-wise. The actions for multiblocks were slightly rearranged. Several new onXRightClicked methods were added to IMetaTileEntity to support the various multi toggle buttons (hard hammer and jack hammer).

These are the main multiblock actions:

  • When a screwdriver is used, the mode is changed (already existed, but improved)
  • When a soldering iron is used, output voiding is toggled (new, migrated from gt++ & bartworksmultis)
  • When wire cutters are used, batch mode is toggled (new, migrated from gt++ & bartworks multis)
  • When a hard hammer is used, input separation is toggled (new, migrated from gt++ & bartworks multis)
  • When a jack hammer is used, recipe locking is toggled (new, migrated from gt++ & bartworks multis)

Singleblock actions:

  • When a hard hammer is used, allow input / allow output is toggled (existing, but its purpose was unclear - chat messages have been improved)
  • When a screwdriver is used, input from output side is toggled (existing)
  • When a soldering iron is used, multiple stacks will be allowed in (existing)
  • When wire cutters are used, stack input filtering is toggled (existing, although I'm not sure why this exists - seems like it could break something)

Shared actions:

  • Wrenches & soft hammers work the same
  • Shift right clicking with wire cutters or a soldering iron will try to connect a neighbouring cable to the block (existing)
  • Various idiosyncrasies around cover sides and wrenching sides when using tools

A lot of the changes are from removing the deprecated onXRightClicked methods. The return type of onScrewdriverRightClicked was changed to boolean to match the other right click methods.

Some machineMode code was cleaned up since a few multis relied on the default behaviour of nextMachineMode when they shouldn't have.

The chat messages for miner mode & radius changes (singleblock & multiblock) were also improved a bit.

@RecursivePineapple RecursivePineapple added the refactor For PRs rewritting a part of the code to have a nicer code overall. label Sep 25, 2024
@RecursivePineapple RecursivePineapple requested a review from a team September 25, 2024 01:58
@RecursivePineapple RecursivePineapple marked this pull request as ready for review September 25, 2024 01:58
@YannickMG
Copy link
Contributor

How does this interact with hatches/buses which might have those features, like input separation?

Also, how discoverable are those interactions? If they're standardized we might want that info on the tooltips for the tools themselves.

@RecursivePineapple
Copy link
Contributor Author

@YannickMG

How does this interact with hatches/buses which might have those features, like input separation?

This doesn't touch anything with the modes themselves, it only lets you toggle them. So input separation etc will behave the same. There are a few hatch-specific settings (sorting, 1 stack limiting, recipe map filtering) but I haven't touched those either.

Also, how discoverable are those interactions? If they're standardized we might want that info on the tooltips for the tools themselves.

The multi-specific logic is defined in the multi base class, so multis can override the methods to opt out. They're standardized enough, but it'd need a disclaimer that some multis might not support some tools. I'm going to add a quest for it since there are quite a few caveats.

@boubou19 boubou19 added the ongoing freeze - do not merge PR tagged with this do not meet the requirement to be merged during a freeze. label Sep 25, 2024
@Dream-Master Dream-Master removed the ongoing freeze - do not merge PR tagged with this do not meet the requirement to be merged during a freeze. label Dec 8, 2024
@YannickMG
Copy link
Contributor

YannickMG commented Dec 15, 2024

By the way I had previously "hidden" some information on tool usage for single block in the tooltips for buttons on their UI, which was better than nothing but not great. That text would either need to be removed or be kept up to date: https://github.com/GTNewHorizons/GT5-Unofficial/pull/858/files#diff-0424401ca733037441e2d1b48951a5ae407801a53d4e84d2e87b8d65e51c3868

@RecursivePineapple RecursivePineapple marked this pull request as draft December 22, 2024 08:58
@RecursivePineapple
Copy link
Contributor Author

Drafting this because I need to take another look at all the machines affected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor For PRs rewritting a part of the code to have a nicer code overall.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants