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

EffSendTitle and EffActionBar object support #7563

Open
wants to merge 9 commits into
base: dev/feature
Choose a base branch
from

Conversation

staffell
Copy link

@staffell staffell commented Feb 1, 2025

Description

This PR aims to add object support to sending titles, subtitles, and actionbars.
Also removed anything related to checking for support for timespans.


Target Minecraft Versions: any
Requirements: none
Related Issues: none

Copy link
Contributor

@TheAbsolutionism TheAbsolutionism left a comment

Choose a reason for hiding this comment

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

Just a few coding conventions

src/main/java/ch/njol/skript/effects/EffSendTitle.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffSendTitle.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffSendTitle.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffSendTitle.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffSendTitle.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffSendTitle.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffSendTitle.java Outdated Show resolved Hide resolved
@TheAbsolutionism
Copy link
Contributor

Also

  1. Make sure to resolve requested changes if you have added them in.
  2. Maybe we should look into updating EffActionBar by removing the deprecated usages.

@staffell
Copy link
Author

staffell commented Feb 1, 2025

  1. Maybe we should look into updating EffActionBar by removing the deprecated usages.

What other way to send an actionbar is there? I couldn't find one but maybe I'm just blind.

@TheAbsolutionism
Copy link
Contributor

What other way to send an actionbar is there? I couldn't find one but maybe I'm just blind.

Well, the current deprecations are BaseComponent and the sendMessage
Would have to update to Adventure stuff.
But should see what the team says first, before doing it.

@staffell
Copy link
Author

staffell commented Feb 2, 2025

Would have to update to Adventure stuff.

Wouldn’t that require a paper server? Just seems not worth it to require paper for titles subtitles and action bars (or am I wrong)

@sovdeeth
Copy link
Member

sovdeeth commented Feb 2, 2025

You should ignore those deprecated methods. They are only deprecated to encourage adventure use.

@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Feb 2, 2025
Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

needs tests

src/main/java/ch/njol/skript/effects/EffSendTitle.java Outdated Show resolved Hide resolved
Copy link
Contributor

@TheAbsolutionism TheAbsolutionism left a comment

Choose a reason for hiding this comment

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

I think the tests need to be done in the JUnit environment and with an actual player object.
You could look within EvtPlayerInputTest to help guide you a bit.
Also, broadcasting is not a form of testing.

@staffell
Copy link
Author

staffell commented Feb 4, 2025

Also, broadcasting is not a form of testing.

Mb, the broadcasts were just to help me personally with timing and stuff, I didn't intend for those to be part of the test. I figured just sending the titles/actionbars were what needed to be tested.

@Burbulinis
Copy link
Contributor

You should look at this for the tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants