Assorted Nitpicks [TMS]
Pheotis opened this issue ยท 20 comments
Assorted minor nitpicks found via the current TMS sweep.
This will be edited over time.
- [222175] Under Aesthetic Tweaks in the default config, the comment mentions that
GLOW_INK_SACK
is a valid colour modifier. That feature was removed. - [222176] Under the default config, running on the craft bukkit 1.18 TMS mock, the default text colour for birch signs is white.
- [222175] Both the supported languages list and the link should be updated.
- [222179] Importing from Knarvik's fork prints a user-level warning... is this normal/intended?
- [222191] When a user creates a blank portal without central permissions, it should just default to the lowest network they have perms for (in this case, personal)
- This error appears for the following portal:
Name1 Name2 flags
They kinda look odd, as we use Bungee chatcolors as coloring, while the glowing highlight depend on dyecolor. They might not match. But is this still a wanted feature?
They kinda look odd, as we use Bungee chatcolors as coloring, while the glowing highlight depend on dyecolor. They might not match. But is this still a wanted feature?
It is a feature in my fork, together with being able to specify the colors used for every type of sign individually. I haven't experienced any weird-looking colors myself. In fact, some of the colors are quite nice:
What logging level do you have in the config, is it Levlel.FINE?
Was mostly just wondering if that error was an issue or not.
If it could be an issue, the current log level is fine.
if it isn't an issue, FINE is probably a better level.
It's not an issue, I meant that this "error" will get thrown if you have debugging enabled
If the error says Index already exists
, there is no problem. If it says anything else, there is a problem. It's a consequence of the lazy handling of duplicate indexes.
It is potentially an issue, but not necessarily.
It already has/is Level.FINE.
This is not a specific issue to my fork. It's printed every time createTables() is run after the first run.
If the error says
Index already exists
, there is no problem. If it says anything else, there is a problem. It's a consequence of the lazy handling of duplicate indexes. It is potentially an issue, but not necessarily.It already has/is Level.FINE.
This is not a specific issue to my fork. It's printed every time createTables() is run after the first run.
Ah, I forgot that the instance was already running at Level.FINE
That makes the 4th item a total non-issue; going to mark that part as resolved.
When applying glow ink to the sign, should the portal revert to black as its color // remove default coloring?
When applying glow ink to the sign, should the portal revert to black as its color // remove default coloring?
If it's dyed, it should just have the highlighted version of the dye. If it's not though, you could either force the color to black or display it as-is, though it becomes kind of unreadable if highlighted and not dyed. I guess the hightlighting itself is trying to highlight black no matter which color we display.
Users must be able to use INC_SAC as well, to remove the dye.
I guess the hightlighting itself is trying to highlight black no matter which color we display.
Exactly. It can for example look like this:
So here the sign is colored black, which makes it go to the default coloring which in this case is light blue. It would be better if the line drawer defaulted back to this:
I guess the hightlighting itself is trying to highlight black no matter which color we display.
Exactly. It can for example look like this:
So here the sign is colored black, which makes it go to the default coloring which in this case is light blue. It would be better if the line drawer defaulted back to this:
For my fork, I used the inverse of the dye color for the highlighting, which generally gives some quite nice colors, especially for glowing signs. Could we add that behavior, or at least make it an option?
/**
* Inverts the given color
*
* @param color <p>The color to invert</p>
* @return <p>The inverted color</p>
*/
public static Color invert(Color color) {
return color.setRed(255 - color.getRed()).setGreen(255 - color.getGreen()).setBlue(255 - color.getBlue());
}
/**
* Gets the chat color corresponding to the given color
*
* @param color <p>The color to convert into a chat color</p>
* @return <p>The resulting chat color</p>
*/
public static ChatColor fromColor(Color color) {
return ChatColor.of(String.format("#%02X%02X%02X", color.getRed(), color.getGreen(), color.getBlue()));
}
That would be interesting! Let's see the current options in this version.
signStyle:
defaultForeground: BLACK
defaultBackground: WHITE
# [1: foreground, 2: gate type, 3: grey, 4: background]
pointer: 2
# [1: selected gates coloured, 2: all gates coloured, 3: no gates coloured]
listing: 2
So if we have pointer option number 4, then do the inversion thing for the pointers, if the sign is not black with default coloring
That could work. I really love some of the contrasts it creates, so I don't want to lose it for my server.
For the language point on this thread. It might be a good idea to push that until alpha release
Also for [222175], should the displayed languages always use the full language codes (nb-NO, en-US), or should the shortcuts (en, nn) be used whenever available?
Also for [222175], should the displayed languages always use the full language codes (nb-NO, en-US), or should the shortcuts (en, nn) be used whenever available?
Shortcuts should be used whenever available.
Anyways, as that nitpick is a formatting issue, probably best to delay dealing with that until the alpha release in case any new language translations come in.
As such, I think it is safe to close this issue now.
It seems PlayerAdvancementDoneEvent
exists in the API, but it cannot be cancelled, and it doesn't seem like it can be modified.
Paper has an event that might be used: PaperMC/Paper#978