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

Persistent, Inpersistent(temporary) holograms #96

Merged
merged 5 commits into from
Jun 1, 2024

Conversation

UsainSrht
Copy link
Contributor

No description provided.

Copy link
Contributor

@OakLoaf OakLoaf left a comment

Choose a reason for hiding this comment

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

This looks really good, would be a helpful feature too!

Comment on lines 65 to 75
/**
* Returns a read-only view of the currently loaded persistent holograms.
*
* @return A read-only collection of holograms.
*/
@Override
public @NotNull
@UnmodifiableView Collection<Hologram> getHolograms() {
return this.holograms.values().stream().filter(hologram -> hologram.getData().isPersistent()).toList();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I would personally name this getPersistentHolograms and then revert getAllHolograms to getHolograms.

@OakLoaf
Copy link
Contributor

OakLoaf commented May 21, 2024

Just thought I'd prompt this thought that I just had - would HologramManagerImpl#reloadHolograms cause all non-persistent holograms to be deleted on plugin reload?

@UsainSrht
Copy link
Contributor Author

Just thought I'd prompt this thought that I just had - would HologramManagerImpl#reloadHolograms cause all non-persistent holograms to be deleted on plugin reload?

probably, let me test and resolve it if thats the case

@OakLoaf
Copy link
Contributor

OakLoaf commented May 21, 2024

This PR will resolve #50

@UsainSrht
Copy link
Contributor Author

Just thought I'd prompt this thought that I just had - would HologramManagerImpl#reloadHolograms cause all non-persistent holograms to be deleted on plugin reload?

fixed with 28f18f2

@OakLoaf
Copy link
Contributor

OakLoaf commented May 23, 2024

Awesome, I think the only thing left would be to look at those methods names mentioned above

Copy link
Contributor

@OakLoaf OakLoaf left a comment

Choose a reason for hiding this comment

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

This looks good to me!

@OliverSchlueter OliverSchlueter merged commit 4d19e1d into FancyMcPlugins:main Jun 1, 2024
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.

3 participants