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

Feature: Powder Chest Timer #2204

Open
wants to merge 25 commits into
base: beta
Choose a base branch
from

Conversation

superhize
Copy link
Contributor

What

Add a timer in the style of the Barn Fishing timer that show you when the first chest you uncovered is gonna despawn.

Images

Changelog New Features

  • Powder Chest Timer. - HiZe

@superhize superhize marked this pull request as draft July 9, 2024 23:05
@superhize superhize marked this pull request as ready for review September 21, 2024 17:44
@hannibal002 hannibal002 added this to the Version 0.28 milestone Sep 21, 2024
Copy link

github-actions bot commented Oct 5, 2024

This pull request has conflicts with the base branch "beta". Please resolve those so we can test out your changes.

@github-actions github-actions bot added the Merge Conflicts There are open merge conflicts with the beta branch. label Oct 5, 2024
@CalMWolfs CalMWolfs self-requested a review October 8, 2024 11:05
@jani270 jani270 added the Soon This Pull Request will be merged within the next couple of betas label Oct 8, 2024
@hannibal002 hannibal002 added Soon This Pull Request will be merged within the next couple of betas and removed Soon This Pull Request will be merged within the next couple of betas labels Oct 11, 2024
@github-actions github-actions bot removed the Merge Conflicts There are open merge conflicts with the beta branch. label Oct 13, 2024
Copy link

Conflicts have been resolved! 🎉

Copy link
Collaborator

@CalMWolfs CalMWolfs left a comment

Choose a reason for hiding this comment

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

it all worked fine but i dont think the gui should be locked behind this option that is in a different page of the config, either have it be linked to the enabled option of make a new option for the display for this feature

image


@SubscribeEvent
fun onWorldChange(event: LorenzWorldChangeEvent) {
if (!isEnabled()) return
Copy link
Contributor

Choose a reason for hiding this comment

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

dont add a config check inside the world change event

else -> continue
}

if (sorted.isNotEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

make this be an early return with if (sorted.isEmpty()) continue

if (sorted.isNotEmpty()) {
val chestToConnect = sorted.take(config.drawLineToChestAmount)
if (chestToConnect.isNotEmpty()) {
val (firstPos, firstTime) = chestToConnect.first()
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of checking isNotEmpty and then doing .first(), you can just do .firstOrNull() ?: continue

@hannibal002 hannibal002 added test and removed test labels Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Soon This Pull Request will be merged within the next couple of betas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants