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

#94 the fix for markerTags not showing up on plugin created commandBlocks #97

Merged
merged 1 commit into from Apr 26, 2017

Conversation

Projects
None yet
2 participants
@mrjvs
Contributor

mrjvs commented Apr 26, 2017

No description provided.

@GnaspGames

This comment has been minimized.

Show comment
Hide comment
@GnaspGames

GnaspGames Apr 26, 2017

Owner

This is the implementation for #94

Honestly I'm a little unsure whether marker entities should be created inside plugins. I can see someone setting it without realising that it will create many more than intended if they use a bang command before resetting it.

But I'm happy to let this go and see what happens.

Owner

GnaspGames commented Apr 26, 2017

This is the implementation for #94

Honestly I'm a little unsure whether marker entities should be created inside plugins. I can see someone setting it without realising that it will create many more than intended if they use a bang command before resetting it.

But I'm happy to let this go and see what happens.

@mrjvs mrjvs changed the title from the fix for markerTags not showing up on plugin created commandBlocks to the fix for markerTags not showing up on plugin created commandBlocks #94 Apr 26, 2017

@mrjvs mrjvs changed the title from the fix for markerTags not showing up on plugin created commandBlocks #94 to #94 the fix for markerTags not showing up on plugin created commandBlocks Apr 26, 2017

@mrjvs

This comment has been minimized.

Show comment
Hide comment
@mrjvs

mrjvs Apr 26, 2017

Contributor

@GnaspGames To fix the more than intended in the delay plugin I used the setJSON function that is also suggested in a pullrequest.

Contributor

mrjvs commented Apr 26, 2017

@GnaspGames To fix the more than intended in the delay plugin I used the setJSON function that is also suggested in a pullrequest.

@GnaspGames

This comment has been minimized.

Show comment
Hide comment
@GnaspGames

GnaspGames Apr 26, 2017

Owner

Right, I see now! Your setting the markerTag INSIDE the band command, and needed it to revert back to whatever was before the bang command was called.

Actually that's what happens with the other JSON props like type and auto.

https://github.com/GnaspGames/Smelt/blob/master/BangCommands/Helper.js#L21

This stops JSON properties from persisting for more than one command when used in a band command. I think I would prefer the markerTag to behave the same so that it's consistent.

I've already started adding to this on my local version, so I'll look into it. There won't be a need for .setJSON then.

Owner

GnaspGames commented Apr 26, 2017

Right, I see now! Your setting the markerTag INSIDE the band command, and needed it to revert back to whatever was before the bang command was called.

Actually that's what happens with the other JSON props like type and auto.

https://github.com/GnaspGames/Smelt/blob/master/BangCommands/Helper.js#L21

This stops JSON properties from persisting for more than one command when used in a band command. I think I would prefer the markerTag to behave the same so that it's consistent.

I've already started adding to this on my local version, so I'll look into it. There won't be a need for .setJSON then.

@mrjvs

This comment has been minimized.

Show comment
Hide comment
@mrjvs

mrjvs Apr 26, 2017

Contributor

Great, but not a single markertag is created on plugin created command block even when the markertag is declared before the the bang command.

Contributor

mrjvs commented Apr 26, 2017

Great, but not a single markertag is created on plugin created command block even when the markertag is declared before the the bang command.

@GnaspGames

This comment has been minimized.

Show comment
Hide comment
@GnaspGames

GnaspGames Apr 26, 2017

Owner

Yes, that will be implemented too. I've got this. 👍

Owner

GnaspGames commented Apr 26, 2017

Yes, that will be implemented too. I've got this. 👍

GnaspGames added a commit that referenced this pull request Apr 26, 2017

For PL #97 - no need to check for markerTag.
Also, should add to commands.push() not the module.

GnaspGames added a commit that referenced this pull request Apr 26, 2017

for #94 and #97 - markerTag is not handled inside
bang commands in the same way other json options are. Any markerTag
assigned inside a band command WON'T persist outside of it. But any
markerTag entity assigned before the bang command is called will, so
long as it's not reset inside the band command script.

@GnaspGames GnaspGames merged commit 8a3cdde into GnaspGames:master Apr 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment