[1.11.2]Substituting Gold_Ore doesn't work the same as Iron_ore??

[olrustyeye]

I wanted to change the way the in game minerals work as far as how they drop. IE: I want gold_ore to drop a piece of gold ore rather then the whole block. I also want to add a slight chance of getting a gem like Diamond. 

Heres whats super bizzaroworld. Iron works, Diamond works, but Coal and Gold don't. I haven't tried redstone yet because that has issues of its own. Also interesting it's crashing at the initialization portion of the game.  Anyone know whats wrong? 

Here my current code
https://pastebin.com/W7tTNY8C

 

And the error:
https://pastebin.com/net807Nd

 

As soon as I comment out substitute(Blocks.GOLD_ORE, new ModBlockOre2("oreGold")); everything magically works. (I've also tried commenting out everything BUT gold ore and it still doesn't work.) 

Edited June 3, 2017 by olrustyeye
Added Version

[Jay Avery]

I don't know the exact cause of the error, but the substitution system is generally extremely buggy. It would be much easier to achieve what you're trying to do using events.

[olrustyeye]

Just now, Jay Avery said:

I don't know the exact cause of the error, but the substitution system is generally extremely buggy. It would be much easier to achieve what you're trying to do using events.

To me it looks like there something it doesn't like about the model. 
I'm a bit confused as to how I can use events to achieve that effect. I don't know much about how to use events to be honest.
Thanks for taking a look!

[Jay Avery]

The docs have a basic overview of using events. To edit the drops from a block, you can use HarvestDropsEvent. In the event, you can check for your chosen block(s) and then edit the list of drops (from event#getDrops()) however you like.

[olrustyeye]

7 minutes ago, Jay Avery said:

The docs have a basic overview of using events. To edit the drops from a block, you can use HarvestDropsEvent. In the event, you can check for your chosen block(s) and then edit the list of drops (from event#getDrops()) however you like.

Ooooo I like that!
I'll have to check it out and utilize it!
Side note: I tested somethings further and just broke my IronOre. Buggy is an understatement. I'm thinking it has something to do with Cache/memory. I'm going to try a restart and see if that clears things up a bit. That being said I'll probably use the events for a cleaner code. 

[olrustyeye]

I've got one issue right now,

This is my code:

@Mod.EventBusSubscriber
public class OverrideBlocksEvents{
 
	 
	@SubscribeEvent
	public static void overrideBlocksDrops(HarvestDropsEvent e){
		
		 if(!e.getWorld().isRemote)
	        {
	            if(e.getWorld().getBlockState(e.getPos()).getBlock() instanceof BlockOre)
	            {
	                e.getDrops().add(new ItemStack(Items.DIAMOND));
	            }
	        }

	}

		

I admittedly found this code from someone else, but from what I've read and understood. This should work. I'm checking what the block state is, and in the instance of a Ore Block it should add a diamond to the drops... right? 

 

Edit: I just used a code I personally had made and it now works because I hadn't initialized it before... jeez goes to show you someone else's code isn't always better.
here it is for those wondering:

	@SubscribeEvent
	public void overrideBlocksDrops(HarvestDropsEvent event){
		
		
				
		if (event.getState().getBlock() == Blocks.IRON_ORE){
		 Utils.getLogger().info("IronBlock Accessed. This method is Working");
			event.getDrops().add(new ItemStack(Blocks.DIAMOND_BLOCK, 1));
		}

	}

Just as a sidenote I'd still like to know why the other code doesn't work?

Edited June 3, 2017 by olrustyeye

[olrustyeye]

Hmm..
So I'm having an issue now where I can't REMOVE the block, but I can add. 

event.getDrops().remove(new ItemStack(Blocks.GOLD_ORE));

I think what I'm needing here is an addition check to check if the block dropped would be gold ore right? I'm just not sure where to go. I've told it to remove that drop but it wont. 

Looking at the other methods I'm not sure how to use remove if this isn't how one uses it? 

[Jay Avery]

You're telling it to remove a newly-created ItemStack object. That exact object won't be present in the list, because you've just made it.

 

You're right in thinking you need to check if the drop would be the ore block. You'll need to iterate through the list of drops, and for each element, check whether its item (ItemStack#getItem) is the ItemBlock for the block in question (Item.getItemFromBlock(block)).

[olrustyeye]

	@SubscribeEvent
	public void overrideBlocksDrops(HarvestDropsEvent event){
		
		
		if (event.getState().getBlock() == Blocks.GOLD_ORE){
		 Utils.getLogger().info("IronBlock Accessed. This method is Working");
			
			event.getDrops().add(new ItemStack(Blocks.DIAMOND_BLOCK, 1));
			
			for(int i=0; i<= event.getDrops().size(); i++){
			if(event.getDrops().get(i) == new ItemStack(Blocks.GOLD_ORE)){
			event.getDrops().remove(i);
			
			Utils.getLogger().info("Was Gold Ore Removed?");
			}
		}
   }

I'm getting somewhere! Right now I got an error and I have some idea of why.
I think there is a parsing error perhaps, or again I'm trying to create something new where something already exsists.

 

Here my issue. Get is looking for an itemstack, so I'm trying to parse blocks into an item stack, but in order to do that I need new. Item.getblockfromitem wouldn't work because that makes it an item not a itemstack. 

I'm not sure where to go from here...

 

This is my error but I feel that the problem is probably obvious to those more experienced.
https://pastebin.com/Bd6KKTKK

 

 

Edited June 3, 2017 by olrustyeye

[Jay Avery]

You can't compare ItemStacks using the == operator, it checks for object identity - so a newly created stack will never be == to an existing stack. Instead you need to check the item that's within the stack (using stack#getItem) and compare it to the block's ItemBlock (which you can obtain from Item.getItemFromBlock). Items can be compared using ==, because they are singletons (so there's only ever one gold ore item, for example).

Edited June 3, 2017 by Jay Avery

[olrustyeye]

17 minutes ago, Jay Avery said:

You can't compare ItemStacks using the == operator, it checks for object identity - so a newly created stack will never be == to an existing stack. Instead you need to check the item that's within the stack (using stack#getItem) and compare it to the block's ItemBlock (which you can obtain from Item.getItemFromBlock).

Ooooooooooooo

	if (event.getState().getBlock() == Blocks.GOLD_ORE){
		 Utils.getLogger().info("IronBlock Accessed. This method is Working");
			
			event.getDrops().add(new ItemStack(Blocks.DIAMOND_BLOCK, 1));
			
			for(int i=0; i<= event.getDrops().size(); i++){
			
				
			if(event.getDrops().get(i).getItem() == Item.getItemFromBlock(Blocks.GOLD_ORE)){
			event.getDrops().remove(i);
			
			Utils.getLogger().info("Was Gold Ore Removed?");
			}
			}

I'm still getting a error. I'm not sure what I am doing wrong now... The only thing I can think of is an issue in the for loop. Because removing the i means that the number that was 2 becomes 1 it has an issue when it goes to check for 2 and it's no longer there...
So I did remove "Blocks.GOLD_ORE"
and I still get an error. 

Seriously, thank you so much for helping me. It's no often a community helps a noob with out being a jerk to him. You guys are awesome, this must be cake walk for a lot of you! 
If it's any consolation I am learning a TON!

One huge key here is that it says, IndexOutofBoundsException. I think it's removing the item and then checking for it again. 

Edited June 3, 2017 by olrustyeye

[olrustyeye]

I fixed it by changing the <= to <

WOOHOO! Maybe I'm not a total noob haha. It works I get my diamond block and NOT my gold ore. 
You guys are seriously a freaking amazing community though. 

[Jay Avery]

I was on the 'noob' side of these conversations for many months, so it seems only fair to pay it forward when I can!

 

Quote

The only thing I can think of is an issue in the for loop. Because removing the i means that the number that was 2 becomes 1 it has an issue when it goes to check for 2 and it's no longer there...

Ah yes, you have the right idea! To handle this safely, you'll need to use an Iterator for the list. Here is a little summary of using iterators. Instead of your for loop, create an iterator and then use a while loop, with the condition that the iterator hasNext. Then to get the next item from the list, instead of your get(i), use next on the iterator. That'll return an element of the list (an ItemStack) in the way you expect, and you can check its conditions in the way you currently are, and remove it from the list using remove on the iterator.

 

Edit: Checking the length in that way unfortunately won't work reliably, because of counting through the list at the same time as changing its length. Here is a stackoverflow question addressing the same problem.

Edited June 3, 2017 by Jay Avery

[olrustyeye]

4 minutes ago, Jay Avery said:

I was on the 'noob' side of these conversations for many months, so it seems only fair to pay it forward when I can!

 

Ah yes, you have the right idea! To handle this safely, you'll need to use an Iterator for the list. Here is a little summary of using iterators. Instead of your for loop, create an iterator and then use a while loop, with the condition that the iterator hasNext. Then to get the next item from the list, instead of your get(i), use next on the iterator. That'll return an element of the list (an ItemStack) in the way you expect, and you can check its conditions in the way you currently are, and remove it form the list using remove on the iterator.

 

Edit: Checking the length in that way unfortunately won't work reliably, because of counting through the list at the same time as changing its length. Here is a stackoverflow question addressing the same problem.

Yup, I was just thinking that. It's a band-aid not a solution. The iterator sounds much more reliable. 

[olrustyeye]

		if (event.getState().getBlock() == Blocks.IRON_ORE){
		 Utils.getLogger().info("IronBlock Accessed. This method is Working");
			event.getDrops().add(new ItemStack(Blocks.DIAMOND_BLOCK, 1));
	
			ListIterator drops = event.getDrops().listIterator();
			while(drops.hasNext()){
			Object element = drops.next();
			
			if(element == Item.getItemFromBlock(Blocks.IRON_ORE)){
			event.getDrops().remove(element);
			 Utils.getLogger().info("WE DID IT!");
			}
			}
		}

So I got the whileloop going now,

my issue is I think I'm deleting the element rather then a block in the element. I'm not sure how to tell it to remove what I want it to at this point.  
Actually theres a problem with the If statement because it's not returning me...oh I think I might know what the issue might be. Element is an itemstack not an item...
Still before I go test that. It's not returning true so theres an issue there. 

Edited June 3, 2017 by olrustyeye

[Jay Avery]

You need to check the item of the element in the same way you were with your for loop: the element is an ItemStack so you need to call getItem to see if that matches the block's item.

 

Edit: You really are getting close, don't be too disheartened!

Edited June 3, 2017 by Jay Avery

[Jay Avery]

4 minutes ago, olrustyeye said:

Actually theres a problem with the If statement because it's not returning me...oh I think I might know what the issue might be. Element is an itemstack not an item...
Still before I go test that. It's not returning true so theres an issue there. 

That's the reason it's not returning true, so fixing that will solve your problem. An ItemStack will never be == to an Item, so the if statement will always be false in its current form.

[olrustyeye]

Yes!!! I got it to work. I don't love the fact I'm casting "element" into an itemstack I feel like that can be refined. But I have another issue I encountered. 
This code does NOT work with redstone. It's only removing one of the redstone. It's interesting because it should be checking it over and over until it deletes all of them right? 

Everything else works like a charm!

		if (event.getState().getBlock() == Blocks.LIT_REDSTONE_ORE){
			 Utils.getLogger().info("IronBlock Accessed. This method is Working");
				event.getDrops().add(new ItemStack(Blocks.DIAMOND_BLOCK, 1));
		
				ListIterator drops = event.getDrops().listIterator();
				while(drops.hasNext()){
				Object element = drops.next();
			
				 Utils.getLogger().info(element);
				if(((ItemStack) element).getItem() == Items.REDSTONE){
				event.getDrops().remove(element);
				 Utils.getLogger().info("WE DID IT!");
				}
				}
			}
		if (event.getState().getBlock() == Blocks.REDSTONE_ORE){
			 Utils.getLogger().info("IronBlock Accessed. This method is Working");
				event.getDrops().add(new ItemStack(Blocks.DIAMOND_BLOCK, 1));
		
				ListIterator drops = event.getDrops().listIterator();
				while(drops.hasNext()){
				Object element = drops.next();
			
				 Utils.getLogger().info(element);
				if(((ItemStack) element).getItem() == Items.REDSTONE){
				event.getDrops().remove(element);
				
				 Utils.getLogger().info("WE DID IT!");
				}
				}
			}

hehe well this isn't the ONLY issue actually. I"m also having an issue of trying to access the metadata of for dyes for lapis ore. I think I might be able to figure that one out, but if not I might be back. 

Edited June 3, 2017 by olrustyeye

[olrustyeye]

43 minutes ago, diesieben07 said:

You could get rid of the ugly cast by not using raw types. getDrops returns a List<ItemStack>, therefor listIterator also returns ListIterator<ItemStack>. Raw types are a legacy thing and should never be used in modern Java code.

Also, don't call remove on the list while you are iterating, this will crash. You need to use the remove method on the iterator to remove the current element.

Holy crap... this is powerful stuff!
It's funny how I'm not thinking in Java terms sometimes. I think I remember back in my college days (Now 4 years ago JEEZ) using lists. I totally forgot how easy they are.
Thank you!!!!
This is solved!

 

For those wondering reading this in the future here is the key:
 

		if (event.getState().getBlock() == Blocks.BLOCK_YOU_WANT_TO_EDIT)
		{
				event.getDrops().add(new ItemStack(YourModItemOrBlock.Item_Or_Block, Number_Of_It)); //This adds the item or block you want to 
																									// You want to add to the block.
				ListIterator<ItemStack> drops = event.getDrops().listIterator(); //This Gets the drop and puts it into a list of ItemStacks
				while(drops.hasNext()) // Using a while loop so we don't get out of the bounds of the list
					{
					ItemStack element = drops.next(); //gets the next itemstack in the list
					if(element.getItem() == Block/Item.BLOCK_OR_ITEM_YOU_WANT_TO_EDIT) // Does the list Item also match the item you want gone?
						{					
						drops.remove();	//remove it!
						}
					}
			}

Now that is some pretty beautiful code. Thanks for the help everyone I learned a TON from this little exercise!

[Draco18s]

4 hours ago, diesieben07 said:

For the future, this feature is called generics, and they are much more powerful than what you have seen here.

Generics are fucking awesome and it's the one tool in the toolbox I don't have when working in Unity that I miss (or at least, not to the same degree: there's typed lists and such, but everything needs to be strictly defined at compile time: no contravariance or covaraiance which means that I can't generically define delegate methods).

 

Also, this method declaration is insane:

https://github.com/Draco18s/ReasonableRealism/blob/master/src/main/java/com/draco18s/flowers/OreFlowersBase.java#L204

It can technically be simplified, but not by a whole lot.

[olrustyeye]

2 hours ago, Draco18s said:

Generics are fucking awesome and it's the one tool in the toolbox I don't have when working in Unity that I miss (or at least, not to the same degree: there's typed lists and such, but everything needs to be strictly defined at compile time: no contravariance or covaraiance which means that I can't generically define delegate methods).

 

Also, this method declaration is insane:

https://github.com/Draco18s/ReasonableRealism/blob/master/src/main/java/com/draco18s/flowers/OreFlowersBase.java#L204

It can technically be simplified, but not by a whole lot.

Generics expand to meet the expanding generics? Lol. 
 

[Draco18s]

2 hours ago, olrustyeye said:

Generics expand to meet the expanding generics? Lol.

I wanted a single function that would handle the linkage between "an ore block" and "a flower block" so that the flowers could be used to indicate the presence of ore in the area. Which involves 3 different block states: the ore, the flower, and the desert flower.  And two properties and their corresponding values (for the flower and desert flower respectively).

 

It's the craziest method signature I've ever written.

[jeffryfisher]

1) I don't like the redundancy between lit and unlit redstone. Can't one boolean expression test for both?

 

2) If removing from a list is problematic, then maybe you could build a replacement list that skips the elements you would delete and then swaps lists at the end.