-
Notifications
You must be signed in to change notification settings - Fork 548
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
Cumulative Armor-Scheduling changes (Rainbow Armor + Radiation update) #3189
Conversation
- item setup - research - rainbow armor task Add AbstractArmorTask as a base class for ArmorTask and RainbowArmorTask
Add RainbowArmorPiece Fix a bug where the color would not change for every player at the same time, but rather per-player online (per scheduling interval)
Add a config parameter to modify the rate at which the RainbowArmor changes colors Change small things related to the RainbowArmor
…te other types of rainbow armor
Co-authored-by: TheBusyBiscuit <TheBusyBiscuit@users.noreply.github.com>
Use stream to map dye colors to colors
…n/tasks/RadioactivityTask.java Co-authored-by: Daniel Walsh <walshydev@gmail.com>
…n/listeners/RadioactivityListener.java Co-authored-by: Daniel Walsh <walshydev@gmail.com>
…n/listeners/RadioactivityListener.java Co-authored-by: Daniel Walsh <walshydev@gmail.com>
…es/Radioactivity.java Co-authored-by: J3fftw <44972470+J3fftw1@users.noreply.github.com>
Co-authored-by: J3fftw <44972470+J3fftw1@users.noreply.github.com>
Co-authored-by: J3fftw <44972470+J3fftw1@users.noreply.github.com>
…n/tasks/RadioactivityTask.java Co-authored-by: J3fftw <44972470+J3fftw1@users.noreply.github.com>
Co-authored-by: J3fftw <44972470+J3fftw1@users.noreply.github.com>
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the one comment
@ParametersAreNonnullByDefault | ||
protected void onArmorPieceTick(Player p, SlimefunArmorPiece sfArmorPiece, ItemStack armorPiece) { | ||
for (PotionEffect effect : sfArmorPiece.getPotionEffects()) { | ||
p.removePotionEffect(effect.getType()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this explicitly remove effects of the same type from the player before applying its own? This forcefully overrides higher level same-type effects or longer effects if the effect on the player is of equal or lower level.
We don't want to replace, for example, Jump Boost 5 with Jump Boost 2; similarly, since Spigot now supports multiple PotionEffects of the same type, instead of removing Jump Boost 1 to apply Jump Boost 2 we can just apply Jump Boost 2 and if by the time JB2 is over JB1 still has time left before expiring it will take over on its own
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A fix can be made by making potion effects last updateInterval + 1s
, then they will expire almost immediately after they have been removed.
That's how the radiation system I implemented does it.
…r-task-pr # Conflicts: # src/main/java/io/github/thebusybiscuit/slimefun4/api/items/HashedArmorpiece.java # src/main/java/io/github/thebusybiscuit/slimefun4/core/attributes/Radioactive.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/SlimefunItems.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/RadioactiveItem.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/armor/Parachute.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/electric/gadgets/JetBoots.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/electric/gadgets/Jetpack.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/electric/gadgets/SolarHelmet.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/magical/BeeWings.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/setup/SlimefunItemSetup.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/ArmorTask.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java # src/test/java/io/github/thebusybiscuit/slimefun4/implementation/items/TestRadioactiveItem.java # src/test/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TestArmorTask.java
Kudos, SonarCloud Quality Gate passed! |
…r-task-pr # Conflicts: # src/main/java/io/github/thebusybiscuit/slimefun4/api/items/HashedArmorpiece.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/RadioactiveItem.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/armor/Parachute.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/armor/RainbowArmorPiece.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/electric/gadgets/SolarHelmet.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/RadioactivityListener.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/ArmorTask.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/armor/RadiationTask.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/armor/SolarHelmetTask.java # src/test/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TestArmorTask.java
Formatting will need to be checked, see #3293 |
Continuation of armor task pull request #3189
Kudos, SonarCloud Quality Gate passed! |
if (sfArmorPiece instanceof RainbowArmorPiece && sfArmorPiece.canUse(p, true)) { | ||
updateRainbowArmor(item, (RainbowArmorPiece) sfArmorPiece); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use instanceof casting here instead of casting the sfArmorPiece
again when calling the method?
@Override | ||
protected void onTick() { | ||
// Do nothing | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason to override this?
@Override | ||
protected void onTick() { | ||
// Do nothing | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again is it necessary to override?
|
||
@ParameterizedTest | ||
@DisplayName("Test Radiation and Hazmat Suits") | ||
@MethodSource("cartesianBooleans") | ||
void testRadiactivity(boolean hazmat, boolean radioactiveFire) throws InterruptedException { | ||
Player player = server.addPlayer(); | ||
TestUtilities.awaitProfile(player); | ||
|
||
// Setting the time to noon, to exclude the Solar Helmet check | ||
player.getWorld().setTime(16000); | ||
|
||
ItemGroup itemGroup = TestUtilities.getItemGroup(plugin, "hazmat_suit_test"); | ||
SlimefunItemStack item = new SlimefunItemStack("MOCK_URANIUM_" + String.valueOf(hazmat).toUpperCase(Locale.ROOT) + "_" + String.valueOf(radioactiveFire).toUpperCase(Locale.ROOT), Material.EMERALD, "&aHi, I am deadly"); | ||
new RadioactiveItem(itemGroup, Radioactivity.VERY_DEADLY, item, RecipeType.NULL, new ItemStack[9]).register(plugin); | ||
|
||
player.getInventory().setItemInMainHand(item.clone()); | ||
player.getInventory().setItemInOffHand(new ItemStack(Material.EMERALD_ORE)); | ||
|
||
if (hazmat) { | ||
SlimefunItemStack chestplate = new SlimefunItemStack("MOCK_HAZMAT_SUIT_" + String.valueOf(radioactiveFire).toUpperCase(Locale.ROOT), Material.LEATHER_CHESTPLATE, "&4Hazmat Prototype"); | ||
MockHazmatSuit armor = new MockHazmatSuit(itemGroup, chestplate); | ||
armor.register(plugin); | ||
|
||
player.getInventory().setChestplate(chestplate.clone()); | ||
} | ||
|
||
ArmorTask task = new ArmorTask(radioactiveFire); | ||
task.run(); | ||
|
||
// Check if the Player is suffering from radiation | ||
boolean radiation = player.getActivePotionEffects().containsAll(task.getRadiationEffects()); | ||
Assertions.assertEquals(!hazmat, radiation); | ||
Assertions.assertEquals(!hazmat && radioactiveFire, player.getFireTicks() > 0); | ||
} | ||
|
||
/** | ||
* This returns an {@link Arguments} {@link Stream} of boolean combinations. | ||
* It performs a cartesian product on two boolean sets. | ||
* | ||
* @return a {@link Stream} of {@link Arguments} | ||
*/ | ||
private static Stream<Arguments> cartesianBooleans() { | ||
Stream<Boolean> stream = Stream.of(true, false); | ||
return stream.flatMap(a -> Stream.of(true, false).map(b -> Arguments.of(a, b))); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a plan to re-do the tests with all of the armor and radiation related changes?
|
||
import javax.annotation.Nonnull; | ||
|
||
import org.apache.commons.lang.Validate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import org.apache.commons.lang.Validate; | |
import org.apache.commons.lang.Validate; | |
Validate.notNull(type, "The effect type cannot be null"); | ||
Validate.isTrue(minExposure > 0, "The minimum exposure must be greater than 0."); | ||
Validate.isTrue(level >= 0, "The status effect level must be non-negative."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preconditions
*/ | ||
|
||
public void apply(@Nonnull Player p) { | ||
Validate.notNull(p, "The player cannot be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preconditions
public RainbowArmorPiece(ItemGroup itemGroup, SlimefunItemStack item, RecipeType recipeType, ItemStack[] recipe, DyeColor[] dyeColors) { | ||
super(itemGroup, item, recipeType, recipe, new PotionEffect[0]); | ||
|
||
Validate.notEmpty(dyeColors, "RainbowArmorPiece colors cannot be empty!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preconditions
* Delay between two "runs" of this task in ticks | ||
*/ | ||
public final void schedule(@Nonnull Slimefun plugin, long tickInterval) { | ||
Validate.notNull(plugin, "The plugin instance cannot be null!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preconditions
private RadiationUtils() {} | ||
|
||
public static void clearExposure(@Nonnull Player p) { | ||
Validate.notNull(p, "The player cannot be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preconditions
} | ||
|
||
public static int getExposure(@Nonnull Player p) { | ||
Validate.notNull(p, "The player must not be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preconditions
} | ||
|
||
public static void addExposure(@Nonnull Player p, int exposure) { | ||
Validate.notNull(p, "The player cannot be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preconditions
} | ||
|
||
public static void removeExposure(@Nonnull Player p, int exposure) { | ||
Validate.notNull(p, "The player should not be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preconditions
Handled in #3892 |
This is a cumulative pull requests consisting of: