search

phrack / ShootOFF-legacy

An open source framework to enhance laser dry fire training.
BSD 4-Clause "Original" or "Old" License
30 stars 17 forks source link

AQT target shapes, click to shoot #46

Closed MisterOuchie closed 10 years ago

MisterOuchie commented 10 years ago

AQT icons, polygon defs, gui button defs, and loading

Click to shoot with a menu item toggle.

phrack commented 10 years ago

The menu item should be left out for click-to-shoot IMO. I think it should just be automatically on in debug mode. It's something only devs really need and they should have no problem at all turning on debug mode with the command line switch.

MisterOuchie commented 10 years ago

ah, I didn't realize there was a command line switch for a debug mode.

I am using click-to-shoot for the offset protocol I mentioned to you, though if I could get the center position of a target on the screen I wouldn't need to.

phrack commented 10 years ago

It would be pretty easy to add a method to the plugins API to give the coordinates of a target's centroid. I've opened an issue for that: https://github.com/phrack/ShootOFF/issues/51.

Even without that use case, click-to-shoot would make it was easier to test protocols because you could simulate precise hits in exactly the color you need to test to make sure your protocol works correctly. For example, with Shoot for Score it would make it way easier to see that points are being assigned correctly for the different regions and across laser colors. I've tested it enough to be confident that it works correctly, but this is a perfect case where click-to-shoot for debugging purposes would have made life way easier.

MisterOuchie commented 10 years ago

Excellent. I changed the code again so it only works in debug mode.

phrack commented 10 years ago

There are a couple minor issues here, so I'll need to incorporate this manually in stages then close it. All of your changes are awesome and will make it in, but there are things like transparency and cross-platform issues that need to be tweaked before shoving them onto master.

The ideal software engineering protocol for both commits and pull requests is to keep them small and coherent. Small means it's the smallest atomic change you can make (it improves the code in some way or works towards a feature without breaking anything). This can get so granular that I would have done the targets in one pull request as you did, but I would have committed each separately. Coherent means that everything in the commit/pull is on one specific topic (e.g. fixing a single bug, adding a single feature, etc.). This protocol exists because it makes it easier to understand what a commit/pull does and because it makes it easier to back out a problem. If you have 4 "things" in one commit/pull, but only one is problematic the entire commit is problematic. If they are 4 different commits/pulls you can deal with the one bad one by itself while everything else goes on just fine.

Anyway, no big deal, shouldn't take too long once I really get going.

phrack commented 10 years ago

endLoop ttsx fix incorporated, still need to do targets/click to shoot.

MisterOuchie commented 10 years ago

Sorry about the mess, every commit I made was added to same pull request, which didn't seem correct. Any guidance on how to do it correctly would be appreciated!

I actually have the offset protocol I mentioned previously working, but I didn't dare foist that on you as well.. : )

phrack commented 10 years ago

I've tweaked some guidance in my post above for moving forward. No big deal!

You can see an example of something I tweaked when applying the tts patch: https://github.com/phrack/ShootOFF/commit/41488a735d0608a41a4d0a15c8d5d37a3e250eb1. You were definitely on the right track, but prefacing a field or method with a _ in Python is a way to signal that it's supposed to be internal and the implementation could change at any time. Right now there is no choice but to do it the way you did, but it can still be done safer.

phrack commented 10 years ago

I just merged click to shoot. I've emailed the Appleseed project to see if they are going to make an issue about using their target shapes. The targets themselves are permissive as long as the copyright notice is preserved, but I am attempting to be extra sure about the shapes too.

phrack commented 10 years ago

A Project Appleseed representative told me they are asking around about the targets and asked me to ping them by Tuesday if we don't have an answer yet.

PS. MisterOuchie, you may want to know that I added a method to the training protocol API today to calculate the centroid of a target.

phrack commented 10 years ago

Appleseed targets are the last code change left for 1.3 remaining. The Project Appleseed rep I am in contact with said they think our use of the shapes will be approved, but no answer yet.

phrack commented 10 years ago

OK, we finally have a resolution to the Project Appleseed target issue (tl; dr: we can only use the target you have incorporated and we just have to give credit for it as expected):

"OK - looks like it's a go. Here's what we can do according to our lawyer volunteer:

Phrack can use the D silhouette and the name "Appleseed(TM)", we can ask him to credit the RWVA and note that the target image is "used with permission."

Thus Phrack can provide the following notice in his software: "The Appleseed(TM) AQT silhouette is used by permission from the RWVA. All rights reserved."

So long as Phrack agrees to limit what he takes to that silhouette and agrees to include this notice, no writing is required (IMO)"

phrack commented 10 years ago

The last thing to do was add the drawing of Project Appleseed target shapes. This was done in the following commmits:

92e701c8f627ddda35009c48baa17bd5a8e25c31 a5811b5d84112dc1caf29f819ec3287b0cf53296 300d4f246547a7fa38fbf0279c5c9e7497b89ea0

This pull request is now closed.