Get Block From Hash Map?

This is probably a simple fix, but I can not work out what I am doing wrong.
I have this piece of code.

Code:java

b.setType(blocks.get(player).getType());

I have saved a block in a hash map, and I want to replace b with that block. Normally I would use Material.something, but the block in hashmap keeps changing. So I made the above, but it does not work. Can someone see what needs to be changed?

 

Gamesareme error?

 

There is non, it simply does not replace the block.

 

You store a reference to the block in the hashmap - so when the block changes, the blog in the hashmap (which is just a reference to the first block) changes as well. You need to create a copy of the block to store it.

 

Paste full class please. Also make sure that you arent getting any errors in console or something.

 

Well here is the full class. Didn't really want to post the full one.
Also there is no error in Terminal.

Code:java

 
 
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
 
 
import org.bukkit.ChatColor;
import org.bukkit.Location;
import org.bukkit.Material;
import org.bukkit.block.Block;
import org.bukkit.entity.Player;
import org.bukkit.entity.Projectile;
import org.bukkit.entity.Snowball;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
import org.bukkit.event.entity.ProjectileHitEvent;
import org.bukkit.event.player.PlayerInteractEvent;
import org.bukkit.scheduler.BukkitRunnable;
 
public class paintgun implements Listener{
public HashMap<Player, Block> blocks = new HashMap<Player, Block>();
public List<Player> shot = new ArrayList<Player>();
Main plugin;
public paintgun(Main main) {
plugin = main;
}
 
@SuppressWarnings("deprecation")
@EventHandler
public void onPlayerInteracteEvent(PlayerInteractEvent event){
Player player = event.getPlayer();
if(player.getItemInHand().getType().equals(Material.SAND)){
player.throwSnowball();
}
}
 
@EventHandler
public void onSnowBallEvent(ProjectileHitEvent event){
Projectile pj = event.getEntity();
//if(!(event.getEntity() instanceof Player)){
// return;
//}
if(pj instanceof Snowball){
@SuppressWarnings("deprecation")
Player player = (Player)event.getEntity().getShooter();
Location loc = pj.getLocation().subtract(0, 1, 0);
Block b = loc.getBlock();
blocks.put(player, b);
if(b.equals(Material.AIR)){
return;
}
b.setType(Material.WOOL);
shot.add(player);
Countdown(player, b);
}
}
 
public void Countdown(final Player player, final Block b) {
new BukkitRunnable() {
@Override
public void run() {
try {
shot.remove(player);
player.sendMessage(ChatColor.GRAY + "[" + ChatColor.GOLD + "PXHub" + ChatColor.GRAY + "] " + ChatColor.GREEN + "You Can Use Bat Gun Again!");
b.setType(blocks.get(player).getType());
 
} catch (Exception e) {
player.sendMessage(ChatColor.RED + "Did not work!");
}
}
}.runTaskLater(this.plugin, 20);
}
 
}

 

Gamesareme this is your answer. Store a BlockState and use .update(true) instead, this will force the block to set the type and data to those of the BlockState

 

Gamesareme OH GOD NO

Don't store Player objects in HashMaps or Lists...

Use a UUID object (you can get a UUID from a player with Player.getUniqueId() and a Player from a UUID with getServer().getPlayer(UUID))

 

PandazNWafflez
Why not?
Weak references to player objects + proper precautions (remove object during onDisable, onQuit, etc) are a lot better than UUID objects.

 

AoH_Ruthless No they simply aren't. It's a bad practice to do stuff like that, because there are circumstances in which a player object can be changed (for example SpoutPlugin (and other things) uses (or at least used to use) reflection to change the players into SpoutPlayer objects) which would then render the original Player object useless / cause problems.

 

PandazNWafflez
You tell me it's bad practice then the example you give me of why UUID's are better is due to reflection of the Player Object, a bad practice in itself? Makes no sense to me.

 

AoH_Ruthless Learn to read, that isn't what I said. I said it's a bad practice because there are situations when it can change. I then went on to provide an example, an example which is one of many.

Also reflection of the player object isn't a bad practice, it's just potentially dangerous.

 

PandazNWafflez how is it bad practice, please elaborate

 

PandazNWafflez
Reflection is a bad practice in several cases because

• it breaks the intention of the original design of a class (by allowing private fields to be accessible).
• extremely fragile in the event of API changes
• difficult to debug for errors
• naturally obfuscates your code by ruining navigation and complicating understanding; it becomes like coding in notepad (with excessive use of reflection, of course).

Reflection is also slightly slower than non-reflection, but not much. While there are many good aspects of reflection, it shouldn't be overused. For Bukkit, reflection is unendorsed for the above and other reasons. I don't know if that's the same on Spout because frankly I am unfamiliar with the workings of it.

That's mildly harsh and immature of you. I read your post, and I pointed out a flaw with your example. Instead of trying to insult my literacy in English (which, mind you, isn't bad at all), you could just refute my post (which you did at the end of yours).

 

I think he's talking about strong references, PandazNWafflez a weak reference will allow the garbage collector to remove an object assuming it's no longer referenced in other places.

 

There's a huge difference between using something that could potentially break something when there is no alternative to using something that could potentially break something when there is. Reflection is a necessary evil in most of these cases. Storing a Player is easily avoidable and completely unnecessary.

It is bad practice because there is a chance it could break something in certain situations, and there is a simple and obvious alternative which is safer.

 

PandazNWafflez A random unofficial build can break UUIDs from working, this still has no relevance to these forums at all. Nor the plugin developer. If you want to use an unofficial build, its not the plugin developer's job to fix its problems.

 

PandazNWafflez I can create memory leaks with UUIDs. So how is it safer?

 

Necrodoom SpoutPlugin is a plugin not an unofficial build. GG

fireblast709 You can, but unless you're a dumbass you won't, because there is no reason to modify the UUID field.

 

PandazNWafflez I dont see any problem stated with player objects and SpoutPlugin. Although, good luck making it work with UUIDs, though (plugin does not work for 1.7.x)

 

Necrodoom There are (and have been) no plans to update it due to ObsidianBox, developed by some of the SpoutPlugin developers. I am also pretty sure SpoutPlugin overrides CraftPlayer objects to SpoutPlayer objects (or it used to, maybe it doesn't anymore).

 

PandazNWafflez

Uhm, I think he was referring to not releasing the UUID, Object pair when the player leaves....

Necrodoom

Fair enough, I just do it because I like to be compatible with other builds, but it isn't of this forum's concern.

 

PandazNWafflez talking purely theoretical there. To continue, I can create memory leaks from UUIDs as Player objects. I have explained in other threads that in a decent amount of cases it can be a lot faster to use Player keys (from O(n^2) to O(n) when we need all players from the Map and perform an arbitrary operation on the current CB version) and I have shown that we can safely use them by using weak referencing (WeakHashMap) and proper cleanup. I can provide more efficiency, yet you claim it is bad practice. Once more, please elaborate :3

 

fireblast709 Efficiency has no effect on this discussion really - realistically it will never matter. And if the object is GC'd it can lead to there not being a player object in there when there should be, and then you can't get one because you don't even have the UUID.

 

PandazNWafflez I still have no idea why would your plugin with UUID compaitiblity for a plugin which you wouldnt be able to run on a server with your plugin has any concern for a plugin developer.

You are just throwing out "X may break Y, therefore we should use Z", which basically says nothing at all.

Yes, UUID allows for more functionality for longer term storage, but player objects work just as well for short term (Which makes your scream for "OH GOD NO" completely pointless), And, player object storage may be more effecient when properly used (On the current CB version, that is, subject to change for 1.8), But all this discussion is entirely offtopic and pointless as Gamesareme hasnt stated what he wants to do with the hashmap anyway.

 

PandazNWafflez when your runningtime would scale polynomial, it does matter . And the second part made not much sense, do you know what a WeakReference is and how it behaves?

 

fireblast709 My understanding is that a WeakReference is a reference to the object, but it allows the object to be garbage collected if completely necessary.

Necrodoom But why not make your plugin compatible with this plugin? There's no reason not to and it isn't difficult.

 

Removed some discussion of flaws in unofficial builds.

 

PandazNWafflez Please see what MC version spoutplugin works for. Now tell me how you are supposed to have UUID compatibility with it.

Its basically dead, and has no relevance to object VS uuid discussion.

 

PandazNWafflez it will allow the GC to garbage collect it if there are no other strong references - likevif the player disconnects. It won't GC earlier, unless you do some weird stuff with your server that transcends the scope of this topic by far (basically you would be doing stuff you shouldn't do in any circumstance).

This is why WeakHashMaps with Player keys work fine. You will never stop the GC from garbage collecting the player when needed, thus no memory leaks should arise.