QualityArmory v3#715
Draft
Lorenzo0111 wants to merge 8 commits into
Draft
Conversation
# Conflicts: # src/main/java/me/zombie_striker/customitemmanager/CustomBaseObject.java # src/main/java/me/zombie_striker/qg/ammo/Ammo.java # src/main/java/me/zombie_striker/qg/armor/ArmorObject.java # src/main/java/me/zombie_striker/qg/listener/QAListener.java # src/main/java/me/zombie_striker/qg/miscitems/MedKit.java # src/main/java/me/zombie_striker/qg/miscitems/MeleeItems.java
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Lorenzo0111
added
enhancement
priority: future
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/me/zombie_striker/qg/listener/QAListener.java (1)
84-89:⚠️ Potential issue | 🟠 MajorClear
ignoreClickwhen the custom interact event is cancelled.Right now a cancelled
QACustomItemInteractEventreturns after the UUID is added, so that player can stay stuck inignoreClickand lose future gun-hit interactions.Suggested fix
- ignoreClick.add(d.getUniqueId()); - QACustomItemInteractEvent event = new QACustomItemInteractEvent(d, g); Bukkit.getPluginManager().callEvent(event); if (event.isCancelled()) return; + ignoreClick.add(d.getUniqueId());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/me/zombie_striker/qg/listener/QAListener.java` around lines 84 - 89, The code adds the player's UUID to ignoreClick then calls QACustomItemInteractEvent and returns if the event is cancelled, leaving the UUID in ignoreClick; update QAListener so that after creating and calling the QACustomItemInteractEvent you check event.isCancelled() and, if true, remove the player's UUID (the same d.getUniqueId() or a stored uuid variable) from ignoreClick before returning, ensuring cancelled events don't leave players stuck in the ignoreClick set.src/main/java/me/zombie_striker/qg/guns/utils/GunUtil.java (1)
101-108:⚠️ Potential issue | 🟠 MajorClamp config-driven step sizes to positive values.
weapons.bulletStepandfeatures.smokeSpacingare used as loop increments. If either is<= 0, shot processing can stall the main thread in the distance / trail loops.Suggested fix
- Vector step = normalizedDirection.clone().multiply(QAMain.getConfiguration().weapons.bulletStep); + double bulletStep = Math.max(QAMain.getConfiguration().weapons.bulletStep, 0.001D); + Vector step = normalizedDirection.clone().multiply(bulletStep); ... - Vector stepSmoke = normalizedDirection.clone().multiply(QAMain.getConfiguration().features.smokeSpacing); - for (double dist = 0; dist < distSqrt; dist += QAMain.getConfiguration().features.smokeSpacing) { + double smokeSpacing = Math.max(QAMain.getConfiguration().features.smokeSpacing, 0.001D); + Vector stepSmoke = normalizedDirection.clone().multiply(smokeSpacing); + for (double dist = 0; dist < distSqrt; dist += smokeSpacing) {Also applies to: 344-345
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/me/zombie_striker/qg/guns/utils/GunUtil.java` around lines 101 - 108, The config-driven loop increments weapons.bulletStep and features.smokeSpacing must be clamped to a positive minimum before use to avoid zero/negative step loops; update GunUtil where you compute Vector step (using QAMain.getConfiguration().weapons.bulletStep) and where smoke spacing is read (the code around the smoke/trail loop referenced at lines ~344-345) to replace raw values with a guarded value like Math.max(configValue, MIN_STEP) (MIN_STEP = a small positive constant, e.g. 1e-6), and use those clamped values when constructing the step Vector and driving the trail/smoke loops and when calling getTargetedSolidMaxDistance so the loops cannot stall on non-positive increments.
🧹 Nitpick comments (6)
src/main/java/me/zombie_striker/customitemmanager/CustomBaseObject.java (1)
262-273: Defensively copy the ingredient array.This base class now sits on the hot path for every custom item, but it still stores and returns the raw
ingarray by reference. Any caller can mutate the recipe after construction, which makes crafting state non-local and fragile.Suggested fix
public void setIngredientsRaw(Object[] ing) { - this.ing = ing; + this.ing = ing == null ? null : ing.clone(); } @@ public Object[] getIngredientsRaw() { - return ing; + return ing == null ? null : ing.clone(); }Also applies to: 305-306
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/me/zombie_striker/customitemmanager/CustomBaseObject.java` around lines 262 - 273, The setters and getters are exposing the internal ingredient array by reference; update setIngredients(ItemStack[] ing) and setIngredientsRaw(Object[] ing) to defensively copy the incoming arrays (e.g., assign a clone/new array and System.arraycopy) and update getIngredientsRaw()/getIngredients() to return a copy rather than the internal this.ing reference so callers cannot mutate internal state; ensure both setters and getters consistently clone the array handling the null case.src/main/java/me/zombie_striker/qg/miscitems/ThrowableItems.java (1)
9-11: MovethrowItemsout of the interface.
throwItemsis global mutable state hanging off a type contract. Now thatThrowableItemsis just an interface, this is a good point to move the registry into a dedicated manager/service so ownership and lifecycle are explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/me/zombie_striker/qg/miscitems/ThrowableItems.java` around lines 9 - 11, The public mutable field throwItems should be removed from the ThrowableItems interface and moved into a dedicated manager class to own lifecycle and access; create a new class (e.g., ThrowableItemRegistry or ThrowableManager) that holds a private Map<Entity, ThrowableHolder> (preferably ConcurrentHashMap) and exposes methods like put(Entity, ThrowableHolder), remove(Entity), get(Entity), contains(Entity) and clear(), then update all usages that referenced ThrowableItems.throwItems to call the new registry methods; ensure the registry field is not public and is the single source of truth so ownership and lifecycle are explicit and thread-safe.src/main/java/me/zombie_striker/qg/miscitems/Molotov.java (1)
29-30: Consider adding braces around theautoarmguard for safety/readability.Current logic is valid, but this nested style is easy to mis-edit later.
♻️ Suggested refactor
- if(!QAMain.getConfiguration().features.autoarm) - if (throwItems.containsKey(thrower)) { + if (!QAMain.getConfiguration().features.autoarm) { + if (throwItems.containsKey(thrower)) { thrower.sendMessage(QAMain.prefix + QAMain.S_GRENADE_PALREADYPULLPIN); thrower.playSound(thrower.getLocation(), WeaponSounds.RELOAD_BULLET.getSoundName(), 1, 1); return true; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/me/zombie_striker/qg/miscitems/Molotov.java` around lines 29 - 30, Wrap the outer guard checking QAMain.getConfiguration().features.autoarm in braces to make the conditional block explicit and prevent accidental mis-editing; specifically, update the if that precedes the existing if (throwItems.containsKey(thrower)) in Molotov.java so the outer if (QAMain.getConfiguration().features.autoarm) { ... } encloses the inner logic (including the throwItems.containsKey(thrower) branch) rather than relying on a single-line conditional.src/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_14/CustomGunItem.java (1)
108-108: ReadITEM_enableUnbreakableonce per item construction.Using a local boolean avoids duplicated config lookups and guarantees both checks use the same value in this method invocation.
♻️ Proposed refactor
- if (QAMain.getConfiguration().items.ITEM_enableUnbreakable) { + boolean enableUnbreakable = QAMain.getConfiguration().items.ITEM_enableUnbreakable; + if (enableUnbreakable) { try { im.setUnbreakable(true); } catch (Error | Exception e34) { } } try { - if (QAMain.getConfiguration().items.ITEM_enableUnbreakable) { + if (enableUnbreakable) { im.addItemFlags(org.bukkit.inventory.ItemFlag.HIDE_UNBREAKABLE); }Also applies to: 115-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_14/CustomGunItem.java` at line 108, Read the ITEM_enableUnbreakable config once at the start of the CustomGunItem construction and reuse that local boolean for all checks in this method (replace repeated QAMain.getConfiguration().items.ITEM_enableUnbreakable calls). Locate the conditional checks in CustomGunItem (the if at the shown line and the related checks around lines 115-116) and introduce a local boolean (e.g., boolean enableUnbreakable = QAMain.getConfiguration().items.ITEM_enableUnbreakable) then use enableUnbreakable for every subsequent branch to avoid duplicate lookups and ensure consistent behavior within the constructor.src/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_13/CustomGunItem.java (1)
68-68: CacheITEM_enableUnbreakableonce in this method.This keeps unbreakable and
HIDE_UNBREAKABLEdecisions consistent and avoids repeated nested config access.♻️ Proposed refactor
- if (QAMain.getConfiguration().items.ITEM_enableUnbreakable) { + boolean enableUnbreakable = QAMain.getConfiguration().items.ITEM_enableUnbreakable; + if (enableUnbreakable) { try { im.setUnbreakable(true); } catch (Error | Exception e34) { } } try { - if (QAMain.getConfiguration().items.ITEM_enableUnbreakable) { + if (enableUnbreakable) { im.addItemFlags(org.bukkit.inventory.ItemFlag.HIDE_UNBREAKABLE); }Also applies to: 75-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_13/CustomGunItem.java` at line 68, Cache QAMain.getConfiguration().items.ITEM_enableUnbreakable into a local boolean at the start of the method in CustomGunItem (so you only call it once) and then use that variable for both the unbreakable check and the HIDE_UNBREAKABLE decision instead of repeatedly accessing QAMain.getConfiguration().items; this ensures consistent decisions for unbreakable and HIDE_UNBREAKABLE (replace the repeated references around the current checks at the lines referencing ITEM_enableUnbreakable and the HIDE_UNBREAKABLE logic).src/main/java/me/zombie_striker/qg/config/GunYMLLoader.java (1)
572-573: Apply verbose logging gate consistently for attachment per-item logs.Line 572 gates the summary, but attachment load lines are still always printed (Line 520), which makes this loader noisier than the others when verbose logging is off.
Suggested consistency patch
- main.getLogger().info("-Loading Attachment: " + name); + if (QAMain.getConfiguration().features.verboseLoadingLogging) { + main.getLogger().info("-Loading Attachment: " + name); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/me/zombie_striker/qg/config/GunYMLLoader.java` around lines 572 - 573, The per-item attachment log statements in GunYMLLoader (the info logs printed around the attachment loading loop, e.g., the main.getLogger().info calls that print each attachment as they load) are not gated by the verboseLoadingLogging flag while the summary line uses that flag; wrap the per-item logging in the same conditional (QAMain.getConfiguration().features.verboseLoadingLogging) so that individual attachment lines are only logged when verboseLoadingLogging is true, leaving the summary line behavior unchanged (the existing check around the "-Loaded "+items+" Attachment types." log can remain as-is).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/me/zombie_striker/qg/config/MainConfig.java`:
- Around line 137-138: The default for impenetrableEntityTypes is immutable
because it's created with Arrays.asList, which causes customMigrate() to throw
UnsupportedOperationException when it tries to append; replace the fixed-size
default with a mutable List instance (e.g., new
ArrayList<>(Arrays.asList(EntityType.ARROW.name()))) so customMigrate() can
safely modify it, and apply the same change to the other similar default list(s)
referenced in the file (the other declaration around the same pattern).
In `@src/main/java/me/zombie_striker/qg/config/system/BaseConfiguration.java`:
- Around line 285-298: The method resolveOrCreateNestedObject currently swallows
all exceptions; change its catch block to report the failure instead of ignoring
it by calling the existing plugin exception handler (e.g.,
plugin.handleException(...)) with a clear message including the field name/type
and the caught exception, so callers still get null but the failure is logged;
keep the rest of resolveOrCreateNestedObject (field.setAccessible,
constructor.newInstance, field.set) unchanged and do not rework normal-return
behavior.
- Around line 197-220: convertRawValue currently lets parsing errors and
serializer.deserialize exceptions propagate and abort reload; wrap all
conversion/parsing paths (primitive branches, boxed type parsing, and the
serializer loop where serializer.deserialize(String.valueOf(raw)) is called) in
try/catch blocks that catch RuntimeException/Exception, log a warning including
the target type and raw value, and return null on failure so a single malformed
on-disk value won’t stop the whole reload/migration; keep using the existing
SERIALIZERS loop and the same type checks (e.g., type.isPrimitive(),
type.equals(Boolean.class), serializer.getType().equals(type)) but protect each
parse/deserialize operation with exception handling.
- Around line 95-97: The code in BaseConfiguration that unconditionally assigns
value = this.getValueList(path, this.resolveListElementType(binding.field)) for
List fields overwrites class defaults with an empty list when the config node is
absent; change the logic in the load path so that for List-typed fields you
first check whether the config actually contains the node (use the existing
config/node existence check used for scalars) and only call getValueList and
assign into the field when the node exists; otherwise leave the existing field
value intact so the class-initialized default list is preserved (refer to
binding.field, getValueList, resolveListElementType and the load/assignment
branch in BaseConfiguration).
In
`@src/main/java/me/zombie_striker/qg/config/system/serializers/EntityTypeSerializer.java`:
- Around line 13-15: The deserializer currently lets IllegalArgumentException
from EntityType.valueOf(from) propagate; update EntityTypeSerializer.deserialize
to defensively handle invalid or null/blank inputs by catching
IllegalArgumentException (and NPE if desired) and returning EntityType.UNKNOWN
as a safe fallback so errors from serializer.deserialize() called in
BaseConfiguration.convertRawValue() no longer abort config loading.
In `@src/main/java/me/zombie_striker/qg/guns/Gun.java`:
- Around line 774-779: The compareTo implementation in Gun uses
(int)(this.getPrice() - arg0.getPrice()), which truncates fractional differences
and can return 0 for distinct prices; change the price comparison to use
Double.compare(this.getPrice(), arg0.getPrice()) (or Long.compare if prices are
integral scaled) when QAMain.getConfiguration().menu.orderShopByPrice is true,
returning that result instead of the casted int, while leaving the fallback to
this.getDisplayName().compareTo(arg0.getDisplayName()) unchanged.
- Around line 974-977: The code is mutating the global flag
QAMain.getConfiguration().weapons.enableIronSightsON_RIGHT_CLICK when a single
player lacks offhand support; instead remove the assignment and treat offhand
support as a per-player check by calling
Update19OffhandChecker.supportOffhand(player.getPlayer()) and, if false, skip or
fall back for that player only (e.g., use a local boolean like canUseIronSights
or consult a per-player map/permission and return/handle without changing the
global config); update the branch in Gun.java where this check occurs to stop
writing to the global config and implement a player-scoped fallback behavior.
In `@src/main/java/me/zombie_striker/qg/guns/projectiles/FireProjectile.java`:
- Line 32: Read weapons.bulletStep once into a local variable in FireProjectile
(e.g., double bulletStep = QAMain.getConfiguration().weapons.bulletStep),
validate that bulletStep > 0 and if not either set a safe default (e.g., 0.1) or
log/error and return, then use that validated value when computing dir2
(dir.clone().multiply(bulletStep)) to prevent an infinite loop when bulletStep
is non-positive.
In `@src/main/java/me/zombie_striker/qg/handlers/BulletWoundHandler.java`:
- Around line 66-67: The equality check on the floating expression bloodlevel /
QAMain.getConfiguration().combat.bulletWound_initialbloodamount == 0.0 is
unreliable; change it to a robust threshold check (e.g. compute denom =
QAMain.getConfiguration().combat.bulletWound_initialbloodamount, guard denom !=
0, then if (bloodlevel / denom <= 0.0 || bloodlevel / denom < EPSILON) or if
(bloodlevel <= SMALL_VALUE) call online.damage(1)). Update the condition around
the online.damage(1) call in BulletWoundHandler to use a non-equality comparison
with a small epsilon instead of == 0.0 and ensure you handle a zero initial
amount to avoid division by zero.
In `@src/main/java/me/zombie_striker/qg/hooks/ViaVersionHook.java`:
- Line 10: The constant VIAVERSION_1_8 in ViaVersionHook is wrong (currently
106); update it to the correct Minecraft 1.8 protocol value by replacing the
hardcoded number with ProtocolVersion.v1_8.getVersion() (or the literal 47) so
comparisons in QAListener and QualityArmory that reference
ViaVersionHook.VIAVERSION_1_8 work correctly; ensure the constant remains public
static final int VIAVERSION_1_8 and import/resolve ProtocolVersion if using the
library accessor.
In `@src/main/java/me/zombie_striker/qg/listener/QAListener.java`:
- Around line 987-989: The extra playSound call after customBase.onSwapTo(...)
causes duplicate equip audio because classes like Gun and MedKit already play
sounds in their onSwapTo implementations; fix by removing the unconditional
playSound call or by guarding it with a capability flag: add a boolean method
(e.g., handlesEquipSound() or shouldAutoPlayEquipSound()) to the CustomBase
class and override it to return true in Gun and MedKit, then update QAListener
to only call getWorld().playSound(...) when customBase.handlesEquipSound() is
false (use existing methods customBase.getSoundOnEquip() and
customBase.getSoundOnEquipVolume() for the actual playback).
In `@src/main/java/me/zombie_striker/qg/miscitems/MedKit.java`:
- Around line 119-122: The severity calculation in MedKit.java uses percentBlood
normalized to 0..1 but compares it to 75/50/25, causing wrong branching; update
the comparisons in the ChatColor severity assignment (the ternary expression
that sets severity) to use fractional thresholds (0.75, 0.5, 0.25) or multiply
percentBlood by 100 before comparing so the branches in the method/class MedKit
produce the intended colors.
---
Outside diff comments:
In `@src/main/java/me/zombie_striker/qg/guns/utils/GunUtil.java`:
- Around line 101-108: The config-driven loop increments weapons.bulletStep and
features.smokeSpacing must be clamped to a positive minimum before use to avoid
zero/negative step loops; update GunUtil where you compute Vector step (using
QAMain.getConfiguration().weapons.bulletStep) and where smoke spacing is read
(the code around the smoke/trail loop referenced at lines ~344-345) to replace
raw values with a guarded value like Math.max(configValue, MIN_STEP) (MIN_STEP =
a small positive constant, e.g. 1e-6), and use those clamped values when
constructing the step Vector and driving the trail/smoke loops and when calling
getTargetedSolidMaxDistance so the loops cannot stall on non-positive
increments.
In `@src/main/java/me/zombie_striker/qg/listener/QAListener.java`:
- Around line 84-89: The code adds the player's UUID to ignoreClick then calls
QACustomItemInteractEvent and returns if the event is cancelled, leaving the
UUID in ignoreClick; update QAListener so that after creating and calling the
QACustomItemInteractEvent you check event.isCancelled() and, if true, remove the
player's UUID (the same d.getUniqueId() or a stored uuid variable) from
ignoreClick before returning, ensuring cancelled events don't leave players
stuck in the ignoreClick set.
---
Nitpick comments:
In `@src/main/java/me/zombie_striker/customitemmanager/CustomBaseObject.java`:
- Around line 262-273: The setters and getters are exposing the internal
ingredient array by reference; update setIngredients(ItemStack[] ing) and
setIngredientsRaw(Object[] ing) to defensively copy the incoming arrays (e.g.,
assign a clone/new array and System.arraycopy) and update
getIngredientsRaw()/getIngredients() to return a copy rather than the internal
this.ing reference so callers cannot mutate internal state; ensure both setters
and getters consistently clone the array handling the null case.
In
`@src/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_13/CustomGunItem.java`:
- Line 68: Cache QAMain.getConfiguration().items.ITEM_enableUnbreakable into a
local boolean at the start of the method in CustomGunItem (so you only call it
once) and then use that variable for both the unbreakable check and the
HIDE_UNBREAKABLE decision instead of repeatedly accessing
QAMain.getConfiguration().items; this ensures consistent decisions for
unbreakable and HIDE_UNBREAKABLE (replace the repeated references around the
current checks at the lines referencing ITEM_enableUnbreakable and the
HIDE_UNBREAKABLE logic).
In
`@src/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_14/CustomGunItem.java`:
- Line 108: Read the ITEM_enableUnbreakable config once at the start of the
CustomGunItem construction and reuse that local boolean for all checks in this
method (replace repeated QAMain.getConfiguration().items.ITEM_enableUnbreakable
calls). Locate the conditional checks in CustomGunItem (the if at the shown line
and the related checks around lines 115-116) and introduce a local boolean
(e.g., boolean enableUnbreakable =
QAMain.getConfiguration().items.ITEM_enableUnbreakable) then use
enableUnbreakable for every subsequent branch to avoid duplicate lookups and
ensure consistent behavior within the constructor.
In `@src/main/java/me/zombie_striker/qg/config/GunYMLLoader.java`:
- Around line 572-573: The per-item attachment log statements in GunYMLLoader
(the info logs printed around the attachment loading loop, e.g., the
main.getLogger().info calls that print each attachment as they load) are not
gated by the verboseLoadingLogging flag while the summary line uses that flag;
wrap the per-item logging in the same conditional
(QAMain.getConfiguration().features.verboseLoadingLogging) so that individual
attachment lines are only logged when verboseLoadingLogging is true, leaving the
summary line behavior unchanged (the existing check around the "-Loaded
"+items+" Attachment types." log can remain as-is).
In `@src/main/java/me/zombie_striker/qg/miscitems/Molotov.java`:
- Around line 29-30: Wrap the outer guard checking
QAMain.getConfiguration().features.autoarm in braces to make the conditional
block explicit and prevent accidental mis-editing; specifically, update the if
that precedes the existing if (throwItems.containsKey(thrower)) in Molotov.java
so the outer if (QAMain.getConfiguration().features.autoarm) { ... } encloses
the inner logic (including the throwItems.containsKey(thrower) branch) rather
than relying on a single-line conditional.
In `@src/main/java/me/zombie_striker/qg/miscitems/ThrowableItems.java`:
- Around line 9-11: The public mutable field throwItems should be removed from
the ThrowableItems interface and moved into a dedicated manager class to own
lifecycle and access; create a new class (e.g., ThrowableItemRegistry or
ThrowableManager) that holds a private Map<Entity, ThrowableHolder> (preferably
ConcurrentHashMap) and exposes methods like put(Entity, ThrowableHolder),
remove(Entity), get(Entity), contains(Entity) and clear(), then update all
usages that referenced ThrowableItems.throwItems to call the new registry
methods; ensure the registry field is not public and is the single source of
truth so ownership and lifecycle are explicit and thread-safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81aef8b7-d813-4635-a5a2-79214d735764
📒 Files selected for processing (50)
pom.xmlsrc/main/java/me/zombie_striker/customitemmanager/ArmoryBaseObject.javasrc/main/java/me/zombie_striker/customitemmanager/CustomBaseObject.javasrc/main/java/me/zombie_striker/customitemmanager/OLD_ItemFact.javasrc/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_13/CustomGunItem.javasrc/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_14/CustomGunItem.javasrc/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_8/CustomGunItem.javasrc/main/java/me/zombie_striker/qg/QAMain.javasrc/main/java/me/zombie_striker/qg/ammo/Ammo.javasrc/main/java/me/zombie_striker/qg/api/QualityArmory.javasrc/main/java/me/zombie_striker/qg/armor/ArmorObject.javasrc/main/java/me/zombie_striker/qg/config/GunYMLLoader.javasrc/main/java/me/zombie_striker/qg/config/MainConfig.javasrc/main/java/me/zombie_striker/qg/config/system/BaseConfiguration.javasrc/main/java/me/zombie_striker/qg/config/system/Config.javasrc/main/java/me/zombie_striker/qg/config/system/serializers/ConfigSerializer.javasrc/main/java/me/zombie_striker/qg/config/system/serializers/EntityTypeSerializer.javasrc/main/java/me/zombie_striker/qg/gui/MenuManager.javasrc/main/java/me/zombie_striker/qg/gui/items/CraftingItem.javasrc/main/java/me/zombie_striker/qg/gui/items/ShopItem.javasrc/main/java/me/zombie_striker/qg/guns/Gun.javasrc/main/java/me/zombie_striker/qg/guns/chargers/BurstFireCharger.javasrc/main/java/me/zombie_striker/qg/guns/chargers/DelayedBurstFireCharger.javasrc/main/java/me/zombie_striker/qg/guns/projectiles/ExplodingRoundProjectile.javasrc/main/java/me/zombie_striker/qg/guns/projectiles/FireProjectile.javasrc/main/java/me/zombie_striker/qg/guns/projectiles/HomingRocketProjectile.javasrc/main/java/me/zombie_striker/qg/guns/projectiles/MiniNukeProjectile.javasrc/main/java/me/zombie_striker/qg/guns/projectiles/RocketProjectile.javasrc/main/java/me/zombie_striker/qg/guns/utils/GunRefillerRunnable.javasrc/main/java/me/zombie_striker/qg/guns/utils/GunUtil.javasrc/main/java/me/zombie_striker/qg/handlers/AimManager.javasrc/main/java/me/zombie_striker/qg/handlers/BlockCollisionUtil.javasrc/main/java/me/zombie_striker/qg/handlers/BulletWoundHandler.javasrc/main/java/me/zombie_striker/qg/handlers/IronsightsHandler.javasrc/main/java/me/zombie_striker/qg/handlers/TreeFellerHandler.javasrc/main/java/me/zombie_striker/qg/handlers/Update19Events.javasrc/main/java/me/zombie_striker/qg/handlers/Update19resourcepackhandler.javasrc/main/java/me/zombie_striker/qg/hooks/ViaVersionHook.javasrc/main/java/me/zombie_striker/qg/listener/QAListener.javasrc/main/java/me/zombie_striker/qg/miscitems/AmmoBag.javasrc/main/java/me/zombie_striker/qg/miscitems/Flashbang.javasrc/main/java/me/zombie_striker/qg/miscitems/Grenade.javasrc/main/java/me/zombie_striker/qg/miscitems/IncendaryGrenades.javasrc/main/java/me/zombie_striker/qg/miscitems/MedKit.javasrc/main/java/me/zombie_striker/qg/miscitems/MeleeItems.javasrc/main/java/me/zombie_striker/qg/miscitems/Molotov.javasrc/main/java/me/zombie_striker/qg/miscitems/ProxyMines.javasrc/main/java/me/zombie_striker/qg/miscitems/SmokeGrenades.javasrc/main/java/me/zombie_striker/qg/miscitems/StickyGrenades.javasrc/main/java/me/zombie_striker/qg/miscitems/ThrowableItems.java
💤 Files with no reviewable changes (1)
- src/main/java/me/zombie_striker/customitemmanager/ArmoryBaseObject.java
There was a problem hiding this comment.
Pull request overview
This PR modernizes QualityArmory for a v3 release by centralizing configuration into a structured MainConfig (reflection + annotations) and removing the legacy ArmoryBaseObject interface in favor of interaction hooks on CustomBaseObject.
Changes:
- Introduces a new annotated configuration system (
BaseConfiguration,@Config, serializers) and replaces manyQAMainstatic config fields withQAMain.getConfiguration().... - Refactors custom item interaction handling by removing
ArmoryBaseObjectand makingCustomBaseObjectthe primary interaction API. - Bumps project version to
3.0.0-SNAPSHOT.
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/me/zombie_striker/qg/QAMain.java | Initializes/uses MainConfig, replaces many legacy static config flags, and updates runtime logic to read from structured config. |
| src/main/java/me/zombie_striker/qg/miscitems/ThrowableItems.java | Removes dependency on the deleted ArmoryBaseObject interface. |
| src/main/java/me/zombie_striker/qg/miscitems/StickyGrenades.java | Switches feature flags (auto-arm/explosion damage) to MainConfig. |
| src/main/java/me/zombie_striker/qg/miscitems/SmokeGrenades.java | Switches feature flags to MainConfig and cleans up imports. |
| src/main/java/me/zombie_striker/qg/miscitems/ProxyMines.java | Switches feature flags to MainConfig. |
| src/main/java/me/zombie_striker/qg/miscitems/Molotov.java | Switches auto-arm flag to MainConfig. |
| src/main/java/me/zombie_striker/qg/miscitems/MeleeItems.java | Removes ArmoryBaseObject usage; relies on CustomBaseObject hooks. |
| src/main/java/me/zombie_striker/qg/miscitems/MedKit.java | Removes ArmoryBaseObject and switches bleeding constants to MainConfig. |
| src/main/java/me/zombie_striker/qg/miscitems/IncendaryGrenades.java | Switches auto-arm flag to MainConfig. |
| src/main/java/me/zombie_striker/qg/miscitems/Grenade.java | Switches feature flags (auto-arm/explosion damage) to MainConfig. |
| src/main/java/me/zombie_striker/qg/miscitems/Flashbang.java | Switches auto-arm flag to MainConfig. |
| src/main/java/me/zombie_striker/qg/miscitems/AmmoBag.java | Removes ArmoryBaseObject usage; relies on CustomBaseObject hooks. |
| src/main/java/me/zombie_striker/qg/listener/QAListener.java | Updates interaction/swap handling to call CustomBaseObject hooks directly and migrates many flags to MainConfig. |
| src/main/java/me/zombie_striker/qg/hooks/ViaVersionHook.java | Extracts ViaVersion 1.8 protocol constant from QAMain. |
| src/main/java/me/zombie_striker/qg/handlers/Update19resourcepackhandler.java | Switches resourcepack flags to MainConfig. |
| src/main/java/me/zombie_striker/qg/handlers/Update19Events.java | Switches resourcepack and reload control flags to MainConfig. |
| src/main/java/me/zombie_striker/qg/handlers/TreeFellerHandler.java | Switches resourcepack override flag to MainConfig. |
| src/main/java/me/zombie_striker/qg/handlers/IronsightsHandler.java | Switches restore-offhand flag to MainConfig and narrows imports. |
| src/main/java/me/zombie_striker/qg/handlers/BulletWoundHandler.java | Switches bleeding toggles/constants to MainConfig. |
| src/main/java/me/zombie_striker/qg/handlers/BlockCollisionUtil.java | Switches bullet-blocking flags to MainConfig. |
| src/main/java/me/zombie_striker/qg/handlers/AimManager.java | Switches sway modifiers to MainConfig. |
| src/main/java/me/zombie_striker/qg/guns/utils/GunUtil.java | Switches weapon/world/ui flags to MainConfig (step, recoil, trails, regen, headshot settings, etc.). |
| src/main/java/me/zombie_striker/qg/guns/utils/GunRefillerRunnable.java | Switches ironsights/UI flags to MainConfig. |
| src/main/java/me/zombie_striker/qg/guns/projectiles/RocketProjectile.java | Switches explosion damage flag to MainConfig. |
| src/main/java/me/zombie_striker/qg/guns/projectiles/MiniNukeProjectile.java | Switches gravity/explosion damage flags to MainConfig. |
| src/main/java/me/zombie_striker/qg/guns/projectiles/HomingRocketProjectile.java | Switches explosion damage flag to MainConfig. |
| src/main/java/me/zombie_striker/qg/guns/projectiles/FireProjectile.java | Switches bullet step to MainConfig. |
| src/main/java/me/zombie_striker/qg/guns/projectiles/ExplodingRoundProjectile.java | Switches gravity to MainConfig. |
| src/main/java/me/zombie_striker/qg/guns/Gun.java | Removes ArmoryBaseObject, updates lore/permission/control logic to MainConfig. |
| src/main/java/me/zombie_striker/qg/guns/chargers/DelayedBurstFireCharger.java | Switches recoil flag to MainConfig. |
| src/main/java/me/zombie_striker/qg/guns/chargers/BurstFireCharger.java | Switches recoil flag to MainConfig. |
| src/main/java/me/zombie_striker/qg/gui/MenuManager.java | Switches shop sorting flag to MainConfig. |
| src/main/java/me/zombie_striker/qg/gui/items/ShopItem.java | Switches permission flags to MainConfig. |
| src/main/java/me/zombie_striker/qg/gui/items/CraftingItem.java | Switches permission flags to MainConfig. |
| src/main/java/me/zombie_striker/qg/config/system/serializers/EntityTypeSerializer.java | Adds an EntityType serializer for the new config system. |
| src/main/java/me/zombie_striker/qg/config/system/serializers/ConfigSerializer.java | Defines the serializer interface for config conversions. |
| src/main/java/me/zombie_striker/qg/config/system/Config.java | Adds @Config annotation for field-path bindings and migrations. |
| src/main/java/me/zombie_striker/qg/config/system/BaseConfiguration.java | Implements reflection-based config loading/saving/migration. |
| src/main/java/me/zombie_striker/qg/config/MainConfig.java | Defines the new structured config schema and migration rules (versioned). |
| src/main/java/me/zombie_striker/qg/config/GunYMLLoader.java | Switches verbose logging flag to MainConfig. |
| src/main/java/me/zombie_striker/qg/armor/ArmorObject.java | Removes ArmoryBaseObject usage; relies on CustomBaseObject hooks. |
| src/main/java/me/zombie_striker/qg/api/QualityArmory.java | Switches UI/resourcepack flags to MainConfig and updates ViaVersion constant usage. |
| src/main/java/me/zombie_striker/qg/ammo/Ammo.java | Removes ArmoryBaseObject usage; relies on CustomBaseObject hooks. |
| src/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_8/CustomGunItem.java | Switches unbreakable flag to MainConfig. |
| src/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_14/CustomGunItem.java | Switches unbreakable flag to MainConfig. |
| src/main/java/me/zombie_striker/customitemmanager/qa/versions/V1_13/CustomGunItem.java | Switches unbreakable flag to MainConfig. |
| src/main/java/me/zombie_striker/customitemmanager/OLD_ItemFact.java | Switches unbreakable flag to MainConfig and cleans imports. |
| src/main/java/me/zombie_striker/customitemmanager/CustomBaseObject.java | Makes CustomBaseObject the primary abstract interaction API (RMB/LMB/shift/swap hooks). |
| src/main/java/me/zombie_striker/customitemmanager/ArmoryBaseObject.java | Deletes the legacy interaction interface. |
| pom.xml | Bumps artifact version to 3.0.0-SNAPSHOT. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
981
to
984
| if (QualityArmory.isCustomItem(prev)) { | ||
| CustomBaseObject customBase = QualityArmory.getCustomItem(prev); | ||
| if (customBase instanceof ArmoryBaseObject) | ||
| ((ArmoryBaseObject) customBase).onSwapAway(e.getPlayer(), prev); | ||
| customBase.onSwapAway(e.getPlayer(), prev); | ||
| } |
There was a problem hiding this comment.
QualityArmory.isCustomItem(prev) can be true for expansion-pack items (see QualityArmory.isCustomItem returning true when QAMain.expansionPacks contains the material), but QualityArmory.getCustomItem(prev) returns null for expansion packs. Calling customBase.onSwapAway(...) without a null-check can therefore throw an NPE. Add a null-check (or explicitly exclude expansion-pack items) before invoking the swap hooks.
Comment on lines
745
to
752
| if (getConfig().contains("SwapSneakToSingleFile")) { | ||
| SwapSneakToSingleFire = (boolean) a("SwapSneakToSingleFire", a("SwapSneakToSingleFile", false)); | ||
| getConfiguration().weapons.SwapSneakToSingleFire = getConfig().getBoolean("SwapSneakToSingleFile", getConfiguration().weapons.SwapSneakToSingleFire); | ||
| getConfig().set("SwapSneakToSingleFile", null); | ||
| getConfig().set("weapons.ironsights.swap-sneak-to-single-fire", getConfiguration().weapons.SwapSneakToSingleFire); | ||
| saveTheConfig = true; | ||
| } | ||
| SwapSneakToSingleFire = (boolean) a("SwapSneakToSingleFire", SwapSneakToSingleFire); | ||
|
|
||
| List<String> destarray = (List<String>) a("DestructableMaterials", | ||
| Collections.singletonList("MATERIAL_NAME_HERE")); | ||
| destructableBlocks.addAll(GunYMLLoader.getMaterials(destarray)); | ||
| regenDestructableBlocksAfter = (int) a("RegenDestructableBlocksAfter", regenDestructableBlocksAfter); | ||
| destructableBlocks.addAll(GunYMLLoader.getMaterials(this.mainConfig.world.destructableMaterials)); | ||
|
|
There was a problem hiding this comment.
destructableBlocks is appended to on every reloadVals() call but is never cleared (unlike interactableBlocks). This can cause duplicates and unnecessary growth over time after repeated reloads. Clear destructableBlocks before adding materials from config.
Comment on lines
+291
to
+297
| @Override | ||
| public void customMigrate() { | ||
| if (this.config.contains("ignoreArmorStands") && this.config.getBoolean("ignoreArmorStands")) { | ||
| if (!weapons.impenetrableEntityTypes.contains(EntityType.ARMOR_STAND.name())) { | ||
| weapons.impenetrableEntityTypes.add(EntityType.ARMOR_STAND.name()); | ||
| } | ||
| } |
There was a problem hiding this comment.
customMigrate() mutates the in-memory weapons.impenetrableEntityTypes list, but BaseConfiguration.migrate() invokes customMigrate() before it copies oldPath values into the new paths. If an existing config already has impenetrableEntityTypes, the subsequent oldPath migration will overwrite the new path and the ArmorStand entry won’t be persisted. Consider performing this migration directly on this.config (read list from old/new path, append if needed, then write it back), or run this logic after the oldPath copy step.
Comment on lines
787
to
792
| expansionPacks.add(MaterialStorage.getMS(Material.DIAMOND_AXE, 116, 0)); | ||
| } | ||
| } else if (true || MANUALLYSELECT14) { | ||
| } else if (true || getConfiguration().versionOverride.MANUALLYSELECT14) { | ||
| //1.14. Use crossbows | ||
| CustomItemManager.registerItemType(getDataFolder(), "gun", new me.zombie_striker.customitemmanager.qa.versions.V1_14.CustomGunItem().setOverrideAttackSpeed((boolean) a("overrideAttackSpeed", true))); | ||
| CustomItemManager.registerItemType(getDataFolder(), "gun", new me.zombie_striker.customitemmanager.qa.versions.V1_14.CustomGunItem().setOverrideAttackSpeed(this.mainConfig.items.overrideAttackSpeed)); | ||
| } |
There was a problem hiding this comment.
The condition else if (true || getConfiguration().versionOverride.MANUALLYSELECT14) is always true and is effectively just an else. Keeping true || is misleading and makes the version-selection logic harder to follow; consider replacing it with a plain else (or the intended boolean expression).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
Version Bump: Updated to v3.0.0-SNAPSHOT (major version)
Refactor: Completely restructured the configuration system. All settings are now centralized through a unified configuration management system instead of scattered static fields, improving organization and maintainability.
API Changes: Internal API refactored with removal of deprecated interfaces and updates to base object handling.