Bedrock Text + Paperweight clean setup #87

Olzie-12 · 2024-05-14T12:34:13Z

Hello, this pull request fixes the paperweight issue of setting up the versions in multiple modules. instead setup the version once with apply false. Updated paper weight to 1.7.1

Bedrock text, server owners now may specify bedrock_text in holograms.yml, and if a player is a geyser user. it'll show this text. Please note I haven't created any command implementations, and adding, removing and inserting is still changing the base text. Maybe something like this should make it implemented to make it more completed.

Antbrix · 2024-05-15T14:22:39Z

I would like to see this merged, it would be a very nice feature to have.

Agaloth · 2024-05-15T23:39:07Z

I would like to see this merged, it would be a very nice feature to have.

This is still being tested by me as right now it's having some trouble figuring out if a player is on bedrock or not, once fixed we'll update the PR

OakLoaf

Due to the nature of this PR I think this should be behind a feature flag, we don't currently have feature flags in FancyHolograms currently so it's likely that it would be pulled following the addition of them.

OakLoaf · 2024-05-16T10:04:06Z

api/src/main/java/de/oliver/fancyholograms/api/Hologram.java

@@ -185,10 +186,10 @@ public final Component getShownText(@Nullable final Player player) {
        if (!(getData().getTypeData() instanceof TextHologramData textData)) {
            return null;
        }
+        var text = textData.getText();
+        if (GeyserChecker.isGeyserPlayer(player) && !textData.getBedrockText().isEmpty()) text = textData.getBedrockText();


Suggested change

if (GeyserChecker.isGeyserPlayer(player) && !textData.getBedrockText().isEmpty()) text = textData.getBedrockText();

if (GeyserChecker.isGeyserPlayer(player) && !textData.getBedrockText().isEmpty()) {

text = textData.getBedrockText();

}

OakLoaf · 2024-05-16T10:07:25Z

api/build.gradle.kts

@@ -8,6 +8,7 @@ dependencies {
    compileOnly("io.papermc.paper:paper-api:${findProperty("minecraftVersion")}-R0.1-SNAPSHOT")

    compileOnly("de.oliver:FancyLib:${findProperty("fancyLibVersion")}")
+    compileOnly("org.geysermc.geyser:api:${findProperty("geyserVersion")}")


I am unsure if this will support all Geyser distributions - as far as I am aware this api would only work for Geyser Spigot/Paper, I would suggest depending on Floodgate instead.

We're completely dropping this detection system, instead we'll modify the config.yml and ask for bedrock player's prefixes, this is the easiest solution for everyone.

I don't personally think that's a good solution, all Geyser servers should have Floodgate installed anyway so hooking into the FloodgateApi should be preferred

The old geyser dependency can be removed now

OakLoaf · 2024-05-16T10:08:24Z

api/src/main/java/de/oliver/fancyholograms/api/utils/GeyserChecker.java

+        if (player == null) return false;
+
+        try {
+            return GeyserApi.api().connectionByUuid(player.getUniqueId()) != null;


As mentioned above, I recommend using Floodgate instead:

Suggested change

return GeyserApi.api().connectionByUuid(player.getUniqueId()) != null;

return FloodgateApi.getInstance().isFloodgatePlayer(player.getUniqueId());

Added workflows, added geyser prefix instead of hooking because of velocity + standalone.

Olzie-12 · 2024-05-16T17:48:41Z

Unfortunately yeah some servers have floodgate installed on velocity, and then using geyser standalone so then the server would not even know if the player is a bedrock player. I don't know much about geyser so please suggest a better idea for this kinda setup

OakLoaf · 2024-05-16T17:52:19Z

Unfortunately yeah some servers have floodgate installed on velocity, and then using geyser standalone so then the server would not even know if the player is a bedrock player. I don't know much about geyser so please suggest a better idea for this kinda setup

The only solution I can think of for this PR is to use floodgate. Floodgate can (and should) be put on all backend servers, the majority of plugins that offer bedrock support will do this through floodgate, if a server owner doesn't want floodgate on their backend servers that is on them and will cause them to lose features in a handful of bedrock compatible plugins.

Olzie-12 · 2024-05-16T20:00:02Z

Unfortunately yeah some servers have floodgate installed on velocity, and then using geyser standalone so then the server would not even know if the player is a bedrock player. I don't know much about geyser so please suggest a better idea for this kinda setup

The only solution I can think of for this PR is to use floodgate. Floodgate can (and should) be put on all backend servers, the majority of plugins that offer bedrock support will do this through floodgate, if a server owner doesn't want floodgate on their backend servers that is on them and will cause them to lose features in a handful of bedrock compatible plugins.

I completely agree, the way of checking of the players name is not nice whatsoever. I'll make sure to revert back to floodgate and make sure that people use floodgate if they want this implementation

ok added floodgate support

OakLoaf · 2024-05-16T20:37:58Z

.github/workflows/maven.yml

Unsure what the purpose of this action is?

it was just a quick little thing as my Gradle crashed on my computer and I couldn't compile so I used a maven template. you can easily remove this if you'd like from this PR

OakLoaf · 2024-05-16T20:43:12Z

api/build.gradle.kts

@@ -8,6 +8,7 @@ dependencies {
    compileOnly("io.papermc.paper:paper-api:${findProperty("minecraftVersion")}-R0.1-SNAPSHOT")

    compileOnly("de.oliver:FancyLib:${findProperty("fancyLibVersion")}")
+    compileOnly("org.geysermc.geyser:api:${findProperty("geyserVersion")}")


The old geyser dependency can be removed now

OakLoaf · 2024-05-16T20:44:46Z

api/src/main/java/de/oliver/fancyholograms/api/utils/FloodgateChecker.java

+        }
+
+        try {
+            boolean b = FloodgateApi.getInstance().isFloodgateId(player.getUniqueId());


Maybe give this parameter a more descriptive name than b I'd suggest something like isBedrockPlayer or even just bedrockPlayer

OakLoaf · 2024-05-16T20:45:11Z

gradle/wrapper/gradle-wrapper.jar

Any reason for removing this?

no, I think my IDE automatically removed it from git

its back now.

OakLoaf · 2024-05-16T22:59:23Z

I'll have another look at this in the morning :)

OakLoaf

Make sure to add floodgate as a dependency in the paper-plugin.yml

OakLoaf · 2024-05-17T10:10:43Z

api/src/main/java/de/oliver/fancyholograms/api/utils/FloodgateChecker.java

+    public static boolean isBedrockPlayer(Player player) {
+        if (player == null) {
+            return false;
+        }
+
+        try {
+            return FloodgateApi.getInstance().isFloodgateId(player.getUniqueId());
+        } catch (Throwable ignored) {
+            return false;
+        }
+    }


As we only need this for online players it would be better to use isFloodgatePlayer there is potential that this current code wouldn't work with linked accounts.

Suggested change

public static boolean isBedrockPlayer(Player player) {

if (player == null) {

return false;

}

try {

return FloodgateApi.getInstance().isFloodgateId(player.getUniqueId());

} catch (Throwable ignored) {

return false;

}

}

public static boolean isBedrockPlayer(@NotNull Player player) {

return FloodgateApi.getInstance().isFloodgatePlayer(player.getUniqueId());

}

As we only need this for online players it would be better to use isFloodgatePlayer there is potential that this current code wouldn't work with linked accounts.

Suggested change

public static boolean isBedrockPlayer(Player player) {

if (player == null) {

return false;

}

try {

return FloodgateApi.getInstance().isFloodgateId(player.getUniqueId());

} catch (Throwable ignored) {

return false;

}

}

public static boolean isBedrockPlayer(@NotNull Player player) {

return FloodgateApi.getInstance().isFloodgatePlayer(player.getUniqueId());

}

for some reason on testing, with a velocity server and geyser standalone with floodgate on the backend server. this was returning false when indeed the player was joined through geyser on standalone and their uuid became floodgate uuid. not sure why it was returning false.

OakLoaf · 2024-05-17T10:14:11Z

api/src/main/java/de/oliver/fancyholograms/api/Hologram.java

-
-        return ModernChatColorHandler.translate(text, player);
+        var text = textData.getText();
+        if (FloodgateChecker.isBedrockPlayer(player) && !textData.getBedrockText().isEmpty()) {


This code would currently break for servers that do not have floodgate installed, it could be worth implementing FloodgateChecker as an object that is enabled onEnable and then accessed through a getFloodgateChecker method

This code would currently break for servers that do not have floodgate installed, it could be worth implementing FloodgateChecker as an object that is enabled onEnable and then accessed through a getFloodgateChecker method

i believe this won't break, as the floodgate checker class is our own. then we use a dirty try/catch to check if the class exists in the class loader. if it errors then they don't have floodgate. this is a very dirty way and your suggestion is definitely better

OakLoaf · 2024-05-17T22:52:02Z

This pr got me inspired to work on my placeholder plugin, it's just a basic PlaceholderAPI add-on that adds a config file for defining text that bedrock players and java players will see.

I was wondering if this would be more suitable for you guys?

Olzie-12 · 2024-05-18T14:51:13Z

This pr got me inspired to work on my placeholder plugin, it's just a basic PlaceholderAPI add-on that adds a config file for defining text that bedrock players and java players will see.

I was wondering if this would be more suitable for you guys?

i don't know why i didn't think of this from the start, in fact i think a lot of servers would benefit from this as its not just limited to the fancy holograms plugin.

OakLoaf · 2024-05-18T19:36:42Z

This pr got me inspired to work on my placeholder plugin, it's just a basic PlaceholderAPI add-on that adds a config file for defining text that bedrock players and java players will see.
I was wondering if this would be more suitable for you guys?

i don't know why i didn't think of this from the start, in fact i think a lot of servers would benefit from this as its not just limited to the fancy holograms plugin.

I did mention this to someone before the pr was made but I guess it didn't make it to you. Nonetheless I do believe that people would still appreciate and use this PR.

Olzie-12 added 8 commits May 14, 2024 13:18

added support for bedrock

31c1f1c

added support for bedrock

0e6a26c

added support for bedrock

1276ece

added support for bedrock

f2b7fc2

added support for bedrock

ad88a4c

added support for bedrock

495f57e

added support for bedrock

0c7608d

added support for bedrock

f56fb4a

Olzie-12 added 2 commits May 15, 2024 22:49

added support for bedrock

9ea3b52

forgot to change compile only.

d89cf32

OakLoaf self-requested a review May 16, 2024 09:58

OakLoaf suggested changes May 16, 2024

View reviewed changes

Olzie-12 added 4 commits May 16, 2024 18:35

added support for bedrock

0925a9d

added support for bedrock

4b3ca5a

added support for bedrock

b282758

Merge pull request #1 from Olzie-12/action

e4c9e61

Added workflows, added geyser prefix instead of hooking because of velocity + standalone.

Olzie-12 added 6 commits May 16, 2024 21:05

floodgate!

60f0591

floodgate!

73108cc

floodgate!

f86577e

floodgate!

aecaf02

Merge pull request #2 from Olzie-12/action

97032cf

ok added floodgate support

Merge branch 'main' into main

08f2103

OakLoaf suggested changes May 16, 2024

View reviewed changes

Olzie-12 added 2 commits May 16, 2024 23:53

Delete .github/workflows directory

f2e0464

Update FloodgateChecker.java

9aabd5d

Update build.gradle.kts

88b4ff0

OakLoaf suggested changes May 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bedrock Text + Paperweight clean setup #87

Bedrock Text + Paperweight clean setup #87

Olzie-12 commented May 14, 2024

Antbrix commented May 15, 2024

Agaloth commented May 15, 2024

OakLoaf left a comment

OakLoaf May 16, 2024

OakLoaf May 16, 2024

Agaloth May 16, 2024

OakLoaf May 16, 2024

OakLoaf May 16, 2024

OakLoaf May 16, 2024

Olzie-12 commented May 16, 2024

OakLoaf commented May 16, 2024

Olzie-12 commented May 16, 2024

OakLoaf May 16, 2024

Olzie-12 May 16, 2024

OakLoaf May 16, 2024

OakLoaf May 16, 2024

OakLoaf May 16, 2024

Olzie-12 May 16, 2024

Olzie-12 May 16, 2024

OakLoaf commented May 16, 2024

OakLoaf left a comment

OakLoaf May 17, 2024

Olzie-12 May 18, 2024

OakLoaf May 17, 2024

Olzie-12 May 18, 2024

OakLoaf commented May 17, 2024

Olzie-12 commented May 18, 2024

OakLoaf commented May 18, 2024

	return GeyserApi.api().connectionByUuid(player.getUniqueId()) != null;
	return FloodgateApi.getInstance().isFloodgatePlayer(player.getUniqueId());

Bedrock Text + Paperweight clean setup #87

Are you sure you want to change the base?

Bedrock Text + Paperweight clean setup #87

Conversation

Olzie-12 commented May 14, 2024

Antbrix commented May 15, 2024

Agaloth commented May 15, 2024

OakLoaf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Olzie-12 commented May 16, 2024

OakLoaf commented May 16, 2024

Olzie-12 commented May 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OakLoaf commented May 16, 2024

OakLoaf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OakLoaf commented May 17, 2024

Olzie-12 commented May 18, 2024

OakLoaf commented May 18, 2024