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

VoxelGadget2 #3

Open
calda opened this issue Dec 5, 2013 · 9 comments
Open

VoxelGadget2 #3

calda opened this issue Dec 5, 2013 · 9 comments

Comments

@calda
Copy link

calda commented Dec 5, 2013

I, advised by Przerwap, have been working on VoxelGadget2 to replace the original plugin. What would be the course of action to have it officially adopted?

I'm currently in the testing and documentation stages, by the way. VoxelGadget2 is complete in terms of features.

Repo: https://github.com/CalDaBeast/VoxelGadget2
Documentation: http://voxelwiki.com/minecraft/VoxelGadget2

@nristock
Copy link

nristock commented Dec 5, 2013

Well the first step will be a full code review by one of our programmers. - That's probably going to be me since I'll have some time tomorrow evening.
We will discuss any further steps - like pulling the repo into our official account, setting up a build on our CI server, etc. - after that.

@calda
Copy link
Author

calda commented Dec 5, 2013

Sounds good to me!

@MikeMatrix
Copy link
Contributor

Just made a PR for it, to adjust some of the things in the Project.
calda/VoxelGadget2#1

@calda
Copy link
Author

calda commented Dec 7, 2013

Have you started looking at it at all, Mono?

@nristock
Copy link

nristock commented Dec 7, 2013

I did and @MikeMatrix did as well. It doesn't look too bad - some parts could use improvement, though. I'll probably add some line comments to your code when I have time.

Some of the things I noticed:

  • don't use public class properties - use getters and setters wherever possible
  • use final (local) variables wherever possible
  • add spaces between literals and braces (have a look at VoxelGuest's or VoxelSniper's code)
if (true) {
}

instead of

if(true){
}

I'm pretty sure there was something else but I don't remember what it was right now.

@calda
Copy link
Author

calda commented Dec 8, 2013

I fixed all of those problems. Thank god for NetBeans' automatic refactoring.

Also apologies for how sloppy the indention was. I hadn't configured my automatic formatting for NetBeans until today (I just started using it) and I didn't realize how bad it is until Deamon pointed it out.

@MikeMatrix
Copy link
Contributor

Looking over the system it's still rather inflexible.
Currently the only 'flexible' system is the configuration of fixed configuration properties.
I would consider changing that into a minimalistic core API with the actual processing delegated to dynamically registered objects.
I'll brainstorm a little.

@calda
Copy link
Author

calda commented Dec 8, 2013

I initial had it very much like you describe but then I ran into the problem that as I made more and more complicated Modifiers it became hard and harder for every other Modifier know what's actually happening. For example, the initial plan for the override modifier was to just use p.setBlock() and be done with it but then I actually still needed to know what the original AND the override block was so that the toggle mode could successfully toggle between them.
I personally don't see Gadget ever really working well as a core API with dynamic objects purely because of the high level of interactions that complicated modifiers require between themselves.

@MikeMatrix
Copy link
Contributor

So far I can see the following actions required to make something dynamically:

  • Adjusting base position of target
  • Adjusting blocks included in relation to base position
  • Execute code based on blocks included
  • Adjusting inventory

All of that could be simplistically overwritten and just passed along the chain.
It should be possible to make all the features we currently have with such a system and easily extend it by simply adding more modifiers, without ever touching the core API.

I'm still twisting my thoughts here though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants