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

Add/Remove snitches on their placement/destruction #5

Closed
awoo-civ opened this issue Dec 23, 2016 · 21 comments
Closed

Add/Remove snitches on their placement/destruction #5

awoo-civ opened this issue Dec 23, 2016 · 21 comments

Comments

@awoo-civ
Copy link
Contributor

awoo-civ commented Dec 23, 2016

Not sure how feasible this is, but it would greatly improve the user experience as updating the whole snitch list is slow and inefficient.

@Gjum
Copy link
Contributor

Gjum commented Dec 23, 2016

Right-clicking a snitch adds it to the "manual" list. I agree that removing on block break would be useful too.

I also noticed that the newest snitches show up on the last /jalist page most of the time, so I open that one manually, but maybe this can be automated somehow.

Adding on placement has the disadvantage that it's not named in that moment, so reading the last /jalist page on /janame would solve these two problems at once.

@MrLittleKitty
Copy link
Owner

Yeah Gjum already mentioned that the main problem with adding snitches on place is that they will not be named and as such won't be in the Snitch List for snitches from /jalist. That is why I added the feature for right clicking to add them to the manual group. If you want to see the snitch's field immediately, for positioning it correctly, you can right click it. When you're done placing the snitches you can use the normal loading feature to have them added normally.

I do agree that removing a snitch on block break would be useful. The exact semantics on what should happen though will probably be a little more complicated.

@Gjum
Copy link
Contributor

Gjum commented Dec 26, 2016

won't be in the Snitch List for snitches from /jalist

That's incorrect; unnamed snitches actually do appear in /jalist. It is just an inconvenience that the name would not be available to Snitch-Master until the next full /jalist scan.

@awoo-civ
Copy link
Contributor Author

awoo-civ commented Dec 29, 2016

I've thought about this a bunch. As it stands, it would indeed be quite hard to add snitches on their placement because it's difficult/inefficient to tell whether a placed Noteblock/Jukebox is reinforced. However, I believe this could be solved by modifying Juke Alert to display the info (name, group, coords, etc.) of a newly placed snitch. The current snitch placement message would remain unchanged; the info would be added as a hover action, making it easily accessible by Snitch Master and non-intrusive to the players.

The issue of naming the snitch could be solved similarly - by adding the hover info to /janame.

Do you agree? If so, I'll look into adding the functionality to JA.

@MrLittleKitty
Copy link
Owner

@Gjum I somehow missed your comment so apologies for my late reply. I phrased what I was trying to say very badly and I am aware that unnamed snitches appear in /jalist. Now that I am actually trying to rephrase my thought I realize that what I was thinking is actually not even an issue. There really isn't much of a problem with adding the snitch on place, except for it not having a name right after being placed. But then we take into account the following:

@iykHvfIvfR I do very much like that idea and I think it would be a welcome addition to JA. With respect to this mod it is most important that the hover actions for creating a snitch and for using /janame contain the coordinates and world of the snitch they are referring to (along with whatever information is relevant to the intended action). I'll pitch your idea to the Devoted guys in discord and see what they think.

@awoo-civ
Copy link
Contributor Author

awoo-civ commented Jan 3, 2017

@MrLittleKitty It's done. Live on the devoted test server, but not live on the main one yet.

@MrLittleKitty
Copy link
Owner

@iykHvfIvfR Alright I'll get around to it after I finish my work on the Gui upgrades. Thanks for the help!

@MrLittleKitty
Copy link
Owner

This is complete as of the guiUpgrades branch. The code for adding a snitch on its placement is there but needs to be tested once the new version of JukeAlert is on the build server or live on Devoted. Snitches are now removed when they are broken.

@awoo-civ
Copy link
Contributor Author

awoo-civ commented Jan 5, 2017

@MrLittleKitty The code at bbd0f74 compiles but crashes the game on startup:

[...]
net.minecraftforge.fml.common.LoaderExceptionModCrash: Caught exception from Snitch Master (snitchmaster)
Caused by: java.lang.IllegalArgumentException: No enum constant com.gmail.nuclearcat1337.snitch_master.Settings.QuietTimeState.false
at java.lang.Enum.valueOf(Unknown Source)
at com.gmail.nuclearcat1337.snitch_master.Settings$QuietTimeState.valueOf(Settings.java:14)
at com.gmail.nuclearcat1337.snitch_master.SnitchMaster$ObjectParser.parse(SnitchMaster.java:229)
at com.gmail.nuclearcat1337.snitch_master.Settings.loadSettings(Settings.java:77)
at com.gmail.nuclearcat1337.snitch_master.SnitchMaster.initializeSettings(SnitchMaster.java:105)
at com.gmail.nuclearcat1337.snitch_master.SnitchMaster.init(SnitchMaster.java:77)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[...]

@MrLittleKitty
Copy link
Owner

@iykHvfIvfR Oh yeah I forgot about that. Its a problem with the settings file that is an unfortunate side effect from changing the quiet time feature. I'm gonna add in a fix right now but if you delete the quiet time line from your settings file it will fix it also.

@MrLittleKitty
Copy link
Owner

@iykHvfIvfR Should be fixed as of 135750d

@awoo-civ
Copy link
Contributor Author

awoo-civ commented Jan 5, 2017

Neither feature seems to work for me on the Devoted test server.

@MrLittleKitty
Copy link
Owner

MrLittleKitty commented Jan 5, 2017

@iykHvfIvfR Which specific features?

@awoo-civ
Copy link
Contributor Author

awoo-civ commented Jan 5, 2017

Placed snitches don't get added and destroyed ones don't get removed. Does it work for you?

@MrLittleKitty
Copy link
Owner

@iykHvfIvfR I looked into it some more and unfortunately it doesn't look like removing snitches on break is going to be as easy to add as I thought. There is a BreakEvent but it doesn't work client side. (I thought it did because I tested it on single player) Barring that event, I haven't found a way to go about doing it.

As for the adding on place, I haven't had a chance to test the code yet. The new JukeAlert version isn't on the Devoted build server and I really can't be bothered to compile it from the source. So don't expect that to work until the new JukeAlert is running on Devoted.

@awoo-civ
Copy link
Contributor Author

awoo-civ commented Jan 7, 2017

As for the adding on place, I haven't had a chance to test the code yet. The new JukeAlert version isn't on the Devoted build server and I really can't be bothered to compile it from the source. So don't expect that to work until the new JukeAlert is running on Devoted.

It is on the server though. I'm able to parse the hover info with my own code, too.

@awoo-civ
Copy link
Contributor Author

awoo-civ commented Feb 1, 2017

@MrLittleKitty It's been a while, so I'm wondering if you've missed my previous message by chance ;)

@MrLittleKitty
Copy link
Owner

@iykHvfIvfR Yeah sorry I've been quite busy as of late. Hopefully when I get some free time I can get around to it :)

@MrLittleKitty
Copy link
Owner

@iykHvfIvfR Its working as of 242c486

@awoo-civ
Copy link
Contributor Author

awoo-civ commented Feb 3, 2017

@MrLittleKitty Wow, so fast :D

I guess now the 'Manual mode' functionality is unnecessary?

@MrLittleKitty
Copy link
Owner

@iykHvfIvfR Not at all. The ability to manually add snitches that you find during your travels is a necessity.

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

No branches or pull requests

3 participants