Skip to content
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

Projectiles #614

Merged
merged 165 commits into from Jan 29, 2018
Merged

Projectiles #614

merged 165 commits into from Jan 29, 2018

Conversation

@Pr0methean
Copy link
Contributor

@Pr0methean Pr0methean commented Dec 17, 2017

Also implements arrow consumption for ItemBow, updates the entity IDs for projectiles, and stubs out and registers missing projectile classes.

@Pr0methean
Copy link
Contributor Author

@Pr0methean Pr0methean commented Dec 17, 2017

This is a partial fix of #505

import org.bukkit.entity.Zombie;
import org.bukkit.entity.ZombieHorse;
import org.bukkit.entity.ZombieVillager;
import org.bukkit.entity.*;
Copy link
Member

@hibo98 hibo98 Dec 17, 2017

Please remove the wildcard import
See Code Style

Pr0methean added 3 commits Dec 17, 2017
Dispenser arrows only deal 1 damage, and they can be picked up
@mastercoms
Copy link
Member

@mastercoms mastercoms commented Dec 18, 2017

FYI, we'll be waiting to merge this after January 1st, due to the current soft feature freeze.

@Pr0methean
Copy link
Contributor Author

@Pr0methean Pr0methean commented Dec 18, 2017

Thanks for the warning. I'll check again for merge conflicts shortly after Christmas.

@mastercoms mastercoms changed the title Merge the old projectiles branch Projectiles Dec 18, 2017
@Cadiducho Cadiducho mentioned this pull request Jan 27, 2018
3 tasks
@@ -49,15 +52,16 @@ private EventFactory() {
* @return the called event
*/
public static <T extends Event> T callEvent(T event) {
GlowServer server = (GlowServer) Bukkit.getServer();
Server server = Bukkit.getServer();

Copy link
Member

@aramperes aramperes Jan 27, 2018

Why was this change necessary? Bukkit.getServer() is guaranteed to be a GlowServer instance. The only difference I see here are the additional imports.

Or was this done because it broke tests somehow?

Copy link
Contributor Author

@Pr0methean Pr0methean Jan 28, 2018

Yes it did (ServerShim doesn't extend GlowServer).

try {
entity.setParticle(Particle.valueOf(particle));
} catch (IllegalArgumentException e) {
Bukkit.getServer().getLogger().warning(String.format(
Copy link
Member

@mastercoms mastercoms Jan 29, 2018

Could you use log(Level.WARNING, ...)

Copy link
Contributor Author

@Pr0methean Pr0methean Jan 29, 2018

Done in aaadd0e.


public class FireballStore<T extends GlowFireball> extends ProjectileStore<T> {

public static final String IS_INCENDIARY = "X-Glowstone-IsIncendiary";
Copy link
Member

@mastercoms mastercoms Jan 29, 2018

Could you make these private like the other ones? Also, I feel like glowstone:IsIncendiary or if that's not valid, glowstoneIsIncendiary would be better.

Copy link
Contributor Author

@Pr0methean Pr0methean Jan 29, 2018

Made private and changed to "glowstone:IsIncendiary" and "glowstone:ExplosionPowerFloat" in aaadd0e.

aramperes
Copy link
Member

aramperes commented on aaadd0e Jan 29, 2018

You can use GlowServer.logger.log(...) instead.

mastercoms
Copy link
Member

mastercoms commented on aaadd0e Jan 29, 2018

Also, I meant use the log with a supplier (like () -> String.format(...)), sorry that I was not clear on that.

Pr0methean
Copy link
Contributor

Pr0methean commented on aaadd0e Jan 29, 2018

Done both in 3404fa2.

// Not in creative mode and have no arrow
break;
default:
player.getServer().getLogger()
Copy link
Member

@mastercoms mastercoms Jan 29, 2018

GlowServer.logger (or is this for testing) and log(Level, Supplier)

Copy link
Contributor Author

@Pr0methean Pr0methean Jan 29, 2018

Done in f656da2 (also migrated to the Supplier<String> logging function).

package net.glowstone.io.entity;

import java.util.UUID;
import java.util.logging.Level;
Copy link
Member

@mastercoms mastercoms Jan 29, 2018

unused import

Copy link
Contributor Author

@Pr0methean Pr0methean Jan 29, 2018

Fixed in b599a16

import net.glowstone.entity.GlowAreaEffectCloud;
import net.glowstone.inventory.GlowMetaPotion;
import net.glowstone.util.nbt.CompoundTag;
import org.bukkit.Bukkit;
Copy link
Member

@mastercoms mastercoms Jan 29, 2018

unused import

Copy link
Contributor Author

@Pr0methean Pr0methean Jan 29, 2018

Fixed in b599a16

@mastercoms mastercoms merged commit 29106ba into GlowstoneMC:dev Jan 29, 2018
2 checks passed
@Pr0methean Pr0methean mentioned this pull request Jan 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants