• Type: Bug
    • Resolution: Fixed
    • Priority: Minor
    • None
    • Affects Version/s: None

      When beetroot is fully grown, this isnt returning true.
      Tried BEETROOT and BEETROOT_BLOCK

      if(b.getType() == Material.BEETROOT_BLOCK){
      	if(b.getData() == CropState.RIPE.getData()){
      
             }
      }
      

          [SPIGOT-1682] Beetroot CropState

          I've created a pull request into bukkit/master

          Ryan Bennitt added a comment - I've created a pull request into bukkit/master

          Updated the docs on the Crops.set/getState and replaced the attached patch. If @Mykindos is happy that my suggested change to his code will work, I can submit a pull request.

          I tried to refactor NetherWarts further, but there really doesn't seem to be a fully backward compatible way to make them a type of Crops.

          Ryan Bennitt added a comment - Updated the docs on the Crops.set/getState and replaced the attached patch. If @Mykindos is happy that my suggested change to his code will work, I can submit a pull request. I tried to refactor NetherWarts further, but there really doesn't seem to be a fully backward compatible way to make them a type of Crops.

          md_5 added a comment -

          Maybe need to add some docs that beetroot etc may not map to all crop states.

          md_5 added a comment - Maybe need to add some docs that beetroot etc may not map to all crop states.

          Ryan Bennitt added a comment - - edited

          Well I have a solution of sorts, I've attached a patch. Your code would have to change slightly, and I'm assuming you've obtained the block by getting it from the world/chunk...

          {{
          if(b.getType() == Material.BEETROOT_BLOCK){
          BlockState bs = b.getState();
          // Creation of a block state by querying the world/chunk should use the Crops class
          // although if the block was constructed manually it is possible it might just be a MaterialData
          // which would cause failure here
          Crops beetrootState = (Crops)bs.getData();
          if(beetrootState.getState() == CropState.RIPE){

          }
          }
          }}

          Does this suit?

          My solution does allow you to set a beetroot to any of the 8 states allowed for regular crops, but they are mapped to only four of those states. So retrieving the value you just set may not result in the same CropState value.

          I was toying with making NetherWarts a type of Crops, however it has its own version of getState() which returns a NetherWartsState, which would have to be child of CropState in order to allow NetherWarts to extend Crops. In my unit test I've demonstrated that it is possible to construct a nether warts block by using Crops instead, although the world/chunk will always return a NetherWarts class if you get a nether warts block. I still might try to resolve the NetherWarts/Crops combination issue...

          Ryan Bennitt added a comment - - edited Well I have a solution of sorts, I've attached a patch. Your code would have to change slightly, and I'm assuming you've obtained the block by getting it from the world/chunk... {{ if(b.getType() == Material.BEETROOT_BLOCK){ BlockState bs = b.getState(); // Creation of a block state by querying the world/chunk should use the Crops class // although if the block was constructed manually it is possible it might just be a MaterialData // which would cause failure here Crops beetrootState = (Crops)bs.getData(); if(beetrootState.getState() == CropState.RIPE){ } } }} Does this suit? My solution does allow you to set a beetroot to any of the 8 states allowed for regular crops, but they are mapped to only four of those states. So retrieving the value you just set may not result in the same CropState value. I was toying with making NetherWarts a type of Crops, however it has its own version of getState() which returns a NetherWartsState, which would have to be child of CropState in order to allow NetherWarts to extend Crops. In my unit test I've demonstrated that it is possible to construct a nether warts block by using Crops instead, although the world/chunk will always return a NetherWarts class if you get a nether warts block. I still might try to resolve the NetherWarts/Crops combination issue...

          Ryan Bennitt added a comment -

          Conversion from CropState to data is simple, just bit shift to the right: CropState.getData() >> 1

          Conversion from data back to CropState requires a sensible mapping. At its simplest CropState.getByData(Crops.getData()*7/3) would map back to SEEDED, VERY_SMALL, MEDIUM, RIPE. But you could round it a little too give different results. CropState.getByData((Crops.getData()*7+1)/3) would map to SEEDED, VERY_SMALL, TALL, RIPE while CropState.getByData((Crops.getData()*7+2)/3) would map to SEEDED, SMALL, TALL, RIPE. I guess the last option is the most optimal mapping.

          Ryan Bennitt added a comment - Conversion from CropState to data is simple, just bit shift to the right: CropState.getData() >> 1 Conversion from data back to CropState requires a sensible mapping. At its simplest CropState.getByData(Crops.getData()*7/3) would map back to SEEDED, VERY_SMALL, MEDIUM, RIPE. But you could round it a little too give different results. CropState.getByData((Crops.getData()*7+1)/3) would map to SEEDED, VERY_SMALL, TALL, RIPE while CropState.getByData((Crops.getData()*7+2)/3) would map to SEEDED, SMALL, TALL, RIPE. I guess the last option is the most optimal mapping.

          Ryan Bennitt added a comment -

          Kind of similar to the problem we had with tree species and all the different ways various Wood blocks actually set their block data. Simplest would be to make Crops class handle the translation between CropState and actual block data value based on its block type. You end up with a switch statement for the different block types and set the block data accordingly.

          Ryan Bennitt added a comment - Kind of similar to the problem we had with tree species and all the different ways various Wood blocks actually set their block data. Simplest would be to make Crops class handle the translation between CropState and actual block data value based on its block type. You end up with a switch statement for the different block types and set the block data accordingly.

          md_5 added a comment -

          Minecraft treats crop age separately from netherwart and beetroot age.
          Bukkit has a separate mapping for netherwart age, problem is if we used it for beetroot things may get confusing. Guess a third generic age, maybe "FastCrop" should be created.

          md_5 added a comment - Minecraft treats crop age separately from netherwart and beetroot age. Bukkit has a separate mapping for netherwart age, problem is if we used it for beetroot things may get confusing. Guess a third generic age, maybe "FastCrop" should be created.

          Tom Hoogstra added a comment -

          Does the same apply to netherwarts?

          Otherwise, how does minecraft determine if the crop is fully grown to give the appropriate amount of harvest?

          Tom Hoogstra added a comment - Does the same apply to netherwarts? Otherwise, how does minecraft determine if the crop is fully grown to give the appropriate amount of harvest?

          md_5 added a comment -

          The problem is that beetroot only has states SEEDED, GERMINATED, VERY_SMALL and SMALL.
          I'm not sure what we should do about this.

          md_5 added a comment - The problem is that beetroot only has states SEEDED, GERMINATED, VERY_SMALL and SMALL. I'm not sure what we should do about this.

            Assignee:
            Unassigned
            Reporter:
            Tom Hoogstra
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: