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

Survivalist Exosuit #73

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Survivalist Exosuit #73

wants to merge 5 commits into from

Conversation

bernie-g
Copy link
Owner

@bernie-g bernie-g commented Jul 21, 2021

This PR:

  • Adds the survivalist exosuit (gives speed and haste when fully worn)
  • Adds item descriptions and adds colors
  • Updates the mod's credits
  • Removes the /assets folder

@bernie-g bernie-g requested a review from justliliandev July 21, 2021 16:38
@@ -32,9 +32,21 @@ public void addTranslation(TranslationLangEntry text, String name) {
add(text.get().getKey(), name);
}

public void addDescription(TranslationLangEntry text, String name) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

name -> description

"itemGroup.techarium": "Techarium",
"shift.techarium.description": "Hold [LShift] for description",
"shift.techarium.description": "\u00A76\u00A76Hold \u00A7b[LShift] \u00A76for a description",
Copy link
Collaborator

Choose a reason for hiding this comment

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

see TechariumLangProvider

addDescription(LangRegistry.botariumDescription, "The Botarium allows you to grow crops in exchange for energy and a suitable fluid");
addDescription(LangRegistry.arboretumDescription, "The Arboretum allows you to grow saplings in exchange for energy and water");
addDescription(LangRegistry.exchangeDescription, "The Exchange Station allows you to unlock new machines with gold");
addDescription(LangRegistry.machineShiftDescription, "&6Hold &b[LShift] &6for a description");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
addDescription(LangRegistry.machineShiftDescription, "&6Hold &b[LShift] &6for a description");
addDescription(LangRegistry.machineShiftDescription, "Hold &b[LShift] &6for a description");

Not neccasary &6 at start
and rename to shiftDescription

return (Ingredient) this.repairMaterial.get();
}

@OnlyIn(Dist.CLIENT)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@OnlyIn(Dist.CLIENT)

Remove OnlyIn. This method is safe to call on server.
This is generally meant for internal Forge and FML use only and modders should avoid its use whenever possible.-JavaDoc of OnlyIn

this.repairMaterial = new LazyValue(repairMaterialSupplier);
}

public int getDurabilityForSlot(EquipmentSlotType slotIn) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

add @OverRide to all methods

Comment on lines +59 to +65
if (ArmorUtil.isWearingAll(player, SurvivalistExosuitItem.class)) {
Utils.addEffect(player, speedBoost);
Utils.addEffect(player, hasteBoost);
} else {
Utils.removeEffect(player, speedBoost);
Utils.removeEffect(player, hasteBoost);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does not work for entites like zombies or villagers but I don't know how to fix that in a good way.
If the player uses shift+delete button in creative or /clear inventory the potion effects stay since the Armor doesn't tick where not all pieces are worn. Maybe move this code to a PlayerTickEvent-Listener with a check for Phase START or END(I would use END)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest using LivingEquipmentChangeEvent that should work for all other entities aswell and since the effect is permanent it doesn't need to be reapplied every tick

protected final LazyValue<Ingredient> repairMaterial;

public TechariumArmorMaterial(String nameIn, int maxDamageFactorIn, int[] damageReductionAmountsIn, int enchantabilityIn, SoundEvent equipSoundIn, float toughnessIn, float knockbackResistanceIn, Supplier<Ingredient> repairMaterialSupplier) {
this.name = nameIn;
Copy link
Collaborator

Choose a reason for hiding this comment

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

you don't need In behind every parameter because you already use this. for everything.

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