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

Modifications to BlockBasePiston and an Orientation Event #192

Closed
wants to merge 4 commits into from
Closed

Modifications to BlockBasePiston and an Orientation Event #192

wants to merge 4 commits into from

Conversation

Eurymachus
Copy link

tryExtend and canExtend methods modified

  • changed the y >= 255 to y >= world.getHeight()

Added BlockEvent, BlockPlacementEvent and BlockOrientationEvent
Modified BlockPistonBase to post a BlockOrientationEvent int determineOrientation allowing mods to register a handler to intercept this event (that parses world, entity, x, y, z)

Note: this will currently allow Pistons and Logs to work corrently in the LittleBlocksMod without the base edit.

 - tryExtend and canExtend methods modified
   - changed the y >= 255 to y >= world.getHeight()
@tambry
Copy link

tambry commented Sep 30, 2012

Like it, should be added in!

Currently only added to BlockPistonBase
This will allow a hook to override placement orientation of an object

Used in LittleBlocks to override piston orientation in LittleWorld
@Eurymachus
Copy link
Author

Apologies for the dual commits!
I didn't realise that it worked this way :) very clever stuff :D

@Eurymachus
Copy link
Author

Is it likely that this change will be made? If so I'll distribute LittleBlocks without the piston functionality temporarily until someone has the time to review and commit.
E.

@tambry
Copy link

tambry commented Oct 2, 2012

Should be done, one of the guys that works on MCForge apparently said that they are preparing for somekind of big update. Also you should notify them to add it on their IRC: #minecraftforge

@AbrarSyed
Copy link
Member

hmmm.
I think it could be best if you redo your current hook in the form of an event, and use the EventBus. Thats probably why Lex hasn't pulled it yet. He might even recode it a different and commit it himself and simply close this PR. In order to save him work, I really really suggest you convert this to an event.

@Eurymachus
Copy link
Author

Sounds good do you know of some simple explanation for using the event bus?

@tambry
Copy link

tambry commented Oct 2, 2012

There are examples of it in Minecraft Forge, look em up!

@Eurymachus
Copy link
Author

I've based my hook on the observer pattern, but Ill look how the event bus works and see if I can move it over.
E.

@Eurymachus
Copy link
Author

Hi guys!
So I've scowered the code and I can see pretty much how to generate the event and post. But what I need is how the events are implemented from the point of view of a mod. For instance I have my new BlockOrientationEvent within the piston orientation code, how then do I implement that to perform some manipulation on the results of the event, thus manipulating the outcome of the orientation method within the piston class?

 - Added BlockEvent
 - Added BlockOrientationEvent extends BlockEvent
 - Modified BlockPistonBase to post orientation event
@Eurymachus
Copy link
Author

As promised, I've now committed a better (advised) solution, an event that's posted on the orientation of the piston.

This will allow manipulation of the orientation values used in calculating the facing of the piston via a handler.

@tambry
Copy link

tambry commented Oct 3, 2012

SHOULD get now pulled.

@Eurymachus
Copy link
Author

Just had a thought. Should BlockOrientationEvent extend PlayerEvent instead?..... Hmmm.... Difficult to classify, since Blocks shouldn't technically be able to generate an event rather the Player is generating the event..... Hmmmm....... Thoughts?

@tambry
Copy link

tambry commented Oct 3, 2012

Go on the Minecraft Forge wiki and ask, Lex should be able to help you.

@Eurymachus
Copy link
Author

Just to note the above modification will also work for BlockLog orientation

 - BlockPlacementEvent added
 - BlockOrientationEvent extends BlockPlacementEvent
@Eurymachus
Copy link
Author

That should be the final commit -.- I decided that a BlockEvent should come from a Living entity event. Although I suppose it can depend. Maybe the block was placed by a deployer.....

@tambry
Copy link

tambry commented Oct 5, 2012

Still not pulled...

@Eurymachus
Copy link
Author

Last thing cpw said was 'we dont know what we're going to do about that one yet'

@Eurymachus
Copy link
Author

I'm actually going to do what Malfunction did with his onBlockActivated hook. I'll add the event into ForgeHooks

EDIT on second thoughts that hook does quite a different thing

@Eurymachus
Copy link
Author

Can I get a confirmation on this PR please?
I noticed Lex mentioned in another post that he likes clean Pull requests. Would you like me to resubmit the PR as clean?
Eury.

@tambry
Copy link

tambry commented Oct 10, 2012

You should sumbit it clean and then see if it will get pulled.

@Eurymachus
Copy link
Author

Still it would be great to get confirmation before I do the work again. Rather than 'see' if it's pulled, will it get pulled if I resubmit. Doesn't help anyone doing uncecessary work after all.
Eury.

@Myrathi
Copy link

Myrathi commented Oct 10, 2012

Cleaning up a PR so that it's a clean submission is not something I'd class as unnecessary, at all. JMHO.

@taralx
Copy link

taralx commented Oct 10, 2012

You don't have to resubmit the PR. You can just modify the branch to the state you want, and this PR will update. (Use git push -f if you're replacing commits.)

@Eurymachus Eurymachus closed this Oct 10, 2012
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

Successfully merging this pull request may close these issues.

None yet

5 participants