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

Follow up fixes for multiple muffler hatches. #3505

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

AbdielKavash
Copy link
Member

@AbdielKavash AbdielKavash commented Nov 16, 2024

This is a follow up to #3489 fixing several problems with that PR.

  • Logic for pollution output no longer creates unnecessary copies of arrays, instead it picks a random index into the existing list of muffler hatches. See comment below.
  • Said logic is also simplified in case the multi has 0 or 1 muffler hatches (as a large majority of multis in a typical base will) to avoid hatch selection completely.
  • Removed unnecessary array copy when generating info data.
  • Info data now mentions and correctly displays average pollution output also for all other machines that override getInfoData.
  • EBF and MEBF now calculate pollution gas output based on the average pollution output of all mufflers (this currently does not matter, as both require exactly 1 muffler, but it is calculated correctly just in case).

@AbdielKavash AbdielKavash added refactor For PRs rewritting a part of the code to have a nicer code overall. bug fix Fix a bug. Please link it in the PR. labels Nov 16, 2024
@AbdielKavash AbdielKavash requested a review from a team November 16, 2024 09:57
@AbdielKavash
Copy link
Member Author

Decided to just make this work correctly instead of the RNG workaround, don't merge yet please.

@AbdielKavash AbdielKavash marked this pull request as draft November 16, 2024 22:01
@AbdielKavash
Copy link
Member Author

New code now spreads the pollution to be output evenly across all mufflers, instead of picking them at random. This means that a single muffler can now output less than 10,000 units of pollution at a time.

A machine will only output up to 10,000 units of pollution per muffler per tick. This is consistent with previous behavior; and as of right now entirely irrelevant, as no machine actually generates enough pollution that this becomes a meaningful bottleneck, unless one changes their configs.

In order to keep Advanced Muffler filter consumption at the same rate as it was previously, if an Advanced Muffler outputs less than 10,000 units of pollution, it has a (linearly proportional) chance to not damage its filter. Thus, for example, an XLGT with four Advanced Mufflers will attempt to output 2,500 units of pollution from each muffler, causing a 25% chance of damaging each of the filters.

This works correctly even when a combination of regular and advanced mufflers of various tiers is used, for the single person out there crazy enough to do this.

One very small breaking change introduced in this PR: if a multiblock has multiple muffler hatches, and one of them is obstructed, previously the machine would keep running if the only (first in the list) muffler that was used to output pollution was free. With this change, since all mufflers are used to output pollution evenly, if any muffler is blocked the machine will stall. Therefore the player needs to ensure that all mufflers are facing air at all times. This is now the intended functionality, the previous behavior is considered a bug.

@AbdielKavash
Copy link
Member Author

Ready for review now.

@AbdielKavash AbdielKavash marked this pull request as ready for review November 17, 2024 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Fix a bug. Please link it in the PR. 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.

2 participants