Skip to content

Conversation

Copy link

Copilot AI commented Jan 31, 2026

Addresses feedback from PR SamB440#118. ServerRegistrationListener only checked groups config for dynamically-registered servers, missing direct servers config entries.

Changes

  • Check servers config first - Direct server name lookup before pattern matching
  • Fall back to groups config - Maintains existing pattern-based matching behavior
  • Early return on match - Stops processing after finding config in servers section

Behavior

When a server registers dynamically:

  1. Skip if resource packs already assigned
  2. Check servers.<server-name> for direct config match → apply packs and return
  3. Check groups.* for pattern matches (exact-match or contains) → apply packs

This aligns with how loadResourcePacks() processes both config sections during plugin initialization.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • repo.spongepowered.org
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:&#43;HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

@copilot open a new pull request to apply changes based on this feedback


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits January 31, 2026 20:33
Co-authored-by: RobotHanzo <36107150+RobotHanzo@users.noreply.github.com>
Co-authored-by: RobotHanzo <36107150+RobotHanzo@users.noreply.github.com>
Copilot AI changed the title [WIP] Update code according to feedback received Support servers config in ServerRegistrationListener Jan 31, 2026
Copilot AI requested a review from RobotHanzo January 31, 2026 20:36
@RobotHanzo RobotHanzo marked this pull request as ready for review January 31, 2026 20:48
Copilot AI review requested due to automatic review settings January 31, 2026 20:48
@RobotHanzo RobotHanzo merged commit ae70f08 into master Jan 31, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates ServerRegistrationListener to apply resource pack configuration for dynamically-registered servers by checking direct servers.<serverName> entries (in addition to existing groups matching).

Changes:

  • Add direct lookup under servers.<serverName> when a server registers dynamically
  • Keep existing groups pattern/exact matching as a fallback path
  • Return early after a direct servers match (as implemented)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +32 to +34
VelocityConfig servers = plugin.getConfig().getConfig("servers");
if (servers != null) {
VelocityConfig serverConfig = servers.getConfig(serverName);
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

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

VelocityConfig servers is declared here, but later in the method (inside the groups loop) there is also a List<String> servers local variable. Java does not allow redeclaring a local variable name in an inner scope, so this will fail to compile. Rename one of them (e.g., serversConfig for the config section, or serverPatterns for the list).

Suggested change
VelocityConfig servers = plugin.getConfig().getConfig("servers");
if (servers != null) {
VelocityConfig serverConfig = servers.getConfig(serverName);
VelocityConfig serversConfig = plugin.getConfig().getConfig("servers");
if (serversConfig != null) {
VelocityConfig serverConfig = serversConfig.getConfig(serverName);

Copilot uses AI. Check for mistakes.
Comment on lines +43 to +46
plugin.registerResourcePackForServer(config, id, serverName, "server", plugin.getConfig().getBoolean("verify-resource-packs"), serverName, null);
});
return;
}
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

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

The return; after handling a servers.<serverName> match prevents groups configs from ever being applied to dynamically-registered servers that also have a direct servers entry. ForcePackVelocity.loadResourcePacks() loads both groups and servers (it calls addResourcePacks(..., "groups") and then addResourcePacks(..., "servers")), so this introduces inconsistent behavior between startup/reload and dynamic registration. Consider removing the early return (register server packs, then still check groups), or update the initialization path if servers is meant to override groups.

Copilot uses AI. Check for mistakes.
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.

2 participants