GlowstoneMC / Glowstone

A fast, customizable and compatible open source server for Minecraft: Java Edition
https://glowstone.net
Other
1.88k stars 270 forks source link

Merge open Glowstone Pull Requests #6

Closed mastercoms closed 9 years ago

mastercoms commented 9 years ago

And some closed PRs that may be useful

mastercoms commented 9 years ago

We should probably discuss each pull before we merge, especially the closed ones.

deathcap commented 9 years ago

Merged a few more which were straightforward or with only minor issues that can be fixed separately. The remaining open PRs (haven't looked at the closed PRs - but good catch digging those up, many do appear to be viable) mostly will be trickier to merge, requiring more changes. Some quick comments:

mastercoms commented 9 years ago

So can you assign me some PRs to do so that we both don't end up doing the same work?

I think that all API changes/additions to the Spigot API should be done in patches, once we port everything from the Spigot API to Glowkit and fix Glowkit.

The reason why we should also support LilyPad is that Spigot also supports BungeeCord. We can differentiate by supporting both.

Spigot does not include action bar events, though this Spigot plugin includes an API for them, and more. We might want to ask for help and permission from the author if we choose to use his code or use code inspired by his.

In GlowstoneMC/Glowstone#627, they remove the fix issue commit in this commit: https://github.com/CrabeMan/Glowstone/commit/52d8b6d68c959628384b3323c35e7ad208e01d04

ethsmith commented 9 years ago

I might help with this as well if its ok, i have my a hosting company so unlimted free servers :P and I got a java background haha

jimmikaelkael commented 9 years ago

To be honest, while it seems to be a good move I'd prefer to continue the development in the Glowstone repo. I had some code for the nether generation almost ready and other stuff too but I prefer to wait if we can hear about @SpaceManiac.

It should be trivial to merge my PRs provided you do it in the good order.

ethsmith commented 9 years ago

It's best to make sure that Glowstone is in good shape for usage by the community, I prefer Glowstone++'s ideas on how to do things, it should be a very awesome project when we have it all finished :P

jimmikaelkael commented 9 years ago

I'm currently enhancing "Lightning damage and ignites" PR so that it matches vanilla more closely.

mastercoms commented 9 years ago

Awesome! I'm working on Dynamic Recipes.

jimmikaelkael commented 9 years ago

Great! :)

ethsmith commented 9 years ago

Want me to do the whitelist fixes or titles API possibly?

ethsmith commented 9 years ago

Or just suggest one and I will work on it,I didn't look into why those 2 were skipped on the list but I'm sure it was for a good reason

jimmikaelkael commented 9 years ago

Hmmmm, I guess the title API requires some stuff to be merged in Glowkit. We should check too if spigot team has pulled something related to titles in their Bukkit fork.

ethsmith commented 9 years ago

ok will just try to do the whitelist code a maybe some others and just leave the Title API for now

mastercoms commented 9 years ago

Title API is not in Spigot, plugins have their own APIs that they use to display titles on Spigot servers.

Whitelist code is fine.

They were skipped because their priority is lower than others.

ethsmith commented 9 years ago

Ok well I will look into the pr's and decide which ones are priority

jimmikaelkael commented 9 years ago

Anyone currently handling PR #618 Action message support ?

ethsmith commented 9 years ago

Im not doing that one and @mastercoms isn't either i don't think but i am not 100% on that

ethsmith commented 9 years ago

I think i am gonna work on PR #627 if noone else has it or it hasn't already been implemented

jimmikaelkael commented 9 years ago

Ok.

mastercoms commented 9 years ago

Don't work on GlowstoneMC/#627. Dynamic Recipes includes that, which is what I'm working on. I might use some code from GlowstoneMC/#627 if needed.

mastercoms commented 9 years ago

@jimmikaelkael Action message, titles, and tags should be done in 1 PR in my opinion, if someone will do it.

jimmikaelkael commented 9 years ago

Well I'll stay away of those PRs and concentrate on other aspects ;)

ethsmith commented 9 years ago

Ok

deathcap commented 9 years ago

Finished merging all open PRs, split off merging closed PRs into https://github.com/GlowstonePlusPlus/GlowstonePlusPlus/issues/55