Skip to content

Conversation

@dogboy21
Copy link
Member

@dogboy21 dogboy21 commented May 11, 2024

PLEASE READ THE GUIDELINES BEFORE MAKING A CONTRIBUTION

  • Please check if the PR fulfills these requirements
  • The commit message are well described
  • Docs have been added / updated (for features or maybe bugs which were noted). If not, please update the needed documentation here. This is not mandatory
  • All changes have fully been tested
  • What kind of change does this PR introduce? (Bug fix, feature, ...)
    Feature

  • What is the current behavior? (You can also link to an open issue here)
    Add game and junit tests to AP #562 is currently missing tests for many peripherals from AP

  • What is the new behavior (if this is a feature change)?
    This PR implements game tests for the following peripherals:

  • NBT Storage
  • Redstone Integrator
  • Geo Scanner
  • Mod Integration: Botania Flowers
  • Mod Integration: Botania Mana Pools
  • Mod Integration: Botania Mana Spreaders
  • Beacon
  • Note Block
  • maybe more?

ToDos:

  • The Geo Scanner docs list a getScanCooldown method which currently does not exist in AP, so I cannot test it (the test currently uses the "internal" getOperationCooldown("scanBlocks") method)
  • The isOnEnchantedSoil function on Botania Mana Flowers always returns false, even when the flower is placed on enchanted soil. This seems to be the case because the overgrowth variable on a Mana Flower is only set to true during the flower tick (see https://github.com/VazkiiMods/Botania/blob/1.20.x/Xplat/src/main/java/vazkii/botania/api/block_entity/SpecialFlowerBlockEntity.java#L100)
  • The isEmpty method on Botania Mana Pools is currently missing
  • For the Botania Mod Integration, some methods are only available in the 1.20.1 versions of AP, so they're currently commented out in the .lua test files. When merging this to the 1.20.1 branch, these methods should be uncommented
  • Botania Mana Spreaders have a getBounding method returning the coordinates of the block that the spreader is directed towards, shouldn't it be called getBinding then?
  • The isEmpty function on Botania Mana Spreaders does not return if the mana in the spreader is empty but if the item handler of the spreader is empty
  • On Note Blocks, the method changeNoteBy is named somewhat weird. It sets the note value to the given parameter and does not change the note by x amount. Shouldn't it be called setNote or something?

@dogboy21 dogboy21 marked this pull request as ready for review May 18, 2024 18:55
@dogboy21 dogboy21 force-pushed the peripheral-tests branch from 57c39ff to 1954f8e Compare May 21, 2024 14:29
@SirEndii SirEndii merged commit 7421144 into IntelligenceModding:feat/tests May 21, 2024
@SirEndii
Copy link
Member

Gonna revert this. I/we want to address some of the to-do's you mentioned

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.

3 participants