KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
693 stars 229 forks source link

Pressing '1' doesn't toggle AG1 when AGX is installed. #711

Closed Dunbaratu closed 9 years ago

Dunbaratu commented 9 years ago

When I have AGX installed, then pressing the '1' key no longer toggles AG1 anymore. It stays false at all times. Same for AG2, AG3, and so on.

When AGX is NOT installed, it works just fine.

This used to work so we (I?) must have broken something recently, but I have no clue what. If someone else with AGX installed can confirm they have the same problem, then this needs to be on the punchlist for 0.17. Too many users use AGX with kOS for us to release if this doesn't work.

SirDiazo commented 9 years ago

Is this on the release or the development version?

I have not used kOS and AGX recently (been working on something else) but I'll run a quick check tonight.

Dunbaratu commented 9 years ago

It's with the develop branch of the kOS code. Which is why I'm afraid I may have somehow broke it. I just got done with a very major overhaul to large parts of the kOS code (to support functions and proper local variable scoping), so we're in the process of testing out a bunch of existing old scripts to make sure they still work.

SirDiazo commented 9 years ago

Alright, I can't actually test anything until I get home after work. Is there a relatively stable dev version somewhere I can grab when I do to check this out?

What I really need to see is the output_log file. If AGX is seeing the request on the action group at all, it should print a message to the log.

One thing to check is if group 11 works also. Groups 1-10 and 11+ are handled differently as well.

frencrs commented 9 years ago

I noticed this but I think it might be intended, the keys work if you set them in AGX. That would let people set their hot keys for what works for them on a case by case basis rather than having to juggle hot keys across multiple plugins. For instance, someone with precise-node installed would probably not want their AGs to trigger when using the keypad to set values for a node. On Apr 2, 2015 9:37 AM, "Steven Mading" notifications@github.com wrote:

It's with the develop branch of the kOS code. Which is why I'm afraid I may have somehow broke it.

— Reply to this email directly or view it on GitHub https://github.com/KSP-KOS/KOS/issues/711#issuecomment-88901238.

Dunbaratu commented 9 years ago

so maybe the issue is that I just don't understand how to use AGX and how keys work in it?

frencrs commented 9 years ago

I might also just be finding a benefit from unintended behavior. Best way to get rid of a bug is to turn it into a feature haha. On Apr 2, 2015 9:53 AM, "Steven Mading" notifications@github.com wrote:

so maybe the issue is that I just don't understand how to use AGX and how keys work in it?

— Reply to this email directly or view it on GitHub https://github.com/KSP-KOS/KOS/issues/711#issuecomment-88909281.

SirDiazo commented 9 years ago

Just to be clear, with AGX installed the default keybinds are overridden.

The key that will activate an action group shows in the lower left of the main AGX window (the one that lists selected parts and their actions) when that group is selected.

By default, the alpha 1 key (not the keypad 1 key) activated action group 1, but that is something to check.

I doubt it is this based on your comment about AGX working previously, but it can't hurt to check.

Dunbaratu commented 9 years ago

I will check it in a few minutes - have to re-run KSP (I de-installed AGX and have to put it back on).

Dunbaratu commented 9 years ago

Okay, I can't figure it out. Here's a screenshot: ![http://i.imgur.com/FLNHphx.png] clicking on any of the AGX windows there doesn't show me the keybindings. I can only see them in the VAB, and THERE, they claim to be mapping AG1 to alpha 1. But trying any sort of '1' - alpha or keypad, has no effect.

The only thing in the output_log mentioning AGX is this:

(Filename: C:/BuildAgent/work/d63dfc6385190b60/artifacts/StandalonePlayerGenerated/UnityEngineDebug.cpp Line: 49)
AGX EditorSaveToFile FAIL 1 System.NullReferenceException: Object reference not set to an instance of an object
  at EditorLogic.recurseShipList (.Part part, System.Collections.Generic.List`1 shipList) [0x00000] in <filename unknown>:0 
  at EditorLogic.getSortedShipList () [0x00000] in <filename unknown>:0 
  at EditorLogic.get_SortedShipList () [0x00000] in <filename unknown>:0 
  at ActionGroupsExtended.AGXEditor.EditorSaveToNode () [0x00000] in <filename unknown>:0 

I started from a fresh sandbox save to be sure nothing was messed up about any vessels it was loading.

But at any rate if someone else with AGX can verify that "print ag1." is toggling the value when they use '1', then I'll consider this an issue that has nothing to do with kOS and take it to the AGX github.

Dunbaratu commented 9 years ago

Incidentally, I have no clue how to make those AGX popup windows go away. They come up some of the time when I launch a vessel to the launchpad, and when they do there's no option to close them and Blizzy's toolbar button isn't appearing, even though the DLL is there for it, and it is able to make the button for IR appear.

It could be just something on my installation is broken, and if AGX with kOS is working fine for other people then I'll just throw up my hands and ignore the problem as not being a bug in kOS that should prevent release.

SirDiazo commented 9 years ago

edit: This post is a red herring, see my next post.

Okay, there is something going on.

First, all the flight scene options are accessed through the AGX button on the toolbar. As you have blizzy's toolbar installed that is where the AGX button shows. As said toolbar is not present, you can't access the AGX button to show/hide those windows or to bring up the "edit mode" windows (the ones you see in the VAB).

If you uninstall Blizzy's toolbar, the AGX button should show on the stock toolbar.

Either way, left-click the AGX button to show/hide the flight windows, right-click to access the options.

Second, the saved action is not transferring to the flight scene.... crap, I know what is going on.

There has been a change in your current version in how action groups are handled. In the current official version, groups 1 through 10 is still handled by stock ksp, only groups 11+ are handled by AGX.

At some point in your dev version, you've switch to having all groups managed by AGX. This causes this issue because AGX does not track action groups, it tracks individual actions, and the "action groups" you see on the screen are virtual constructs generated as needed.

Because of this, with no actions the action group to activate, AGX will always return a state of false for that group, explaining the behavior you see.

This is the reason I added the "Script Trigger" action to all the kOS computer parts. It is an empty action that does nothing that you can trigger to cause AGX to turn an action group on/off for stuff like this.

Lastly, the "AGX EditorSaveToFile FAIL 1" error you reported is expected when you leave the editor with no parts placed, such as entering the editor, then immediately "leaving" it for another editor scene when you load a different vessel. I don't know enough c# code to bypass that error message at the moment.

Dunbaratu commented 9 years ago

If I uninstall Blizzy's toolbar, I can't use the IR mod. As far as I remember, it only uses Blizzy, not the AppLauncher.

As for this statement: "At some point in your dev version, you've switch to having all groups managed by AGX. This causes this issue because ...", I sadly don't know that code and wasn't involved in that. I'd like someone who was to respond and see if we can get this resolved quickly to get our release out.

SirDiazo commented 9 years ago

Okay, cancel my previous post. I looked at the code and the current dev version is in fact correct. Groups 1 to 10 are handled by KSP and 11+ by AGX.

Note that I believe this behavior is currently in the official released version as well.

At this point I believe this is an AGX issue. The root cause comes back to the fact that without an action in the action group, AGX does not consider that group to exist.

Therefore when you press 1 on the keyboard, AGX ignores the keypress because there are no actions in group 1 and does nothing. However, AGX still blocks the keypress from falling through to default KSP so nothing appears to happen.

To test:

With no action in action group 1: "ag1 on" in the kOS terminal should turn the group on and "print ag1" should return true.

With an action in action group 1: Pressing 1 on the keyboard will behave as expected because the presence of the action causes AGX to activate the action group.

Note that this issue (assuming I am right this time) was known when I originally implemented this, it is the reason the "Script Trigger" action is present on the kOS computer, it gives AGX something to work with for this situation.

Without that Script Trigger action, the action group does not shown in the AGX actions list and may confuse the player when AGX is telling them action group 1 does nothing, while kOS still allows you to toggle action group 1 (the 10 default groups).

(still on my mobile, tests pending when I get home)

SirDiazo commented 9 years ago

Okay, sorry for the double post, but I need you guys to see this so I want to make sure it shows up in your alerts as I believe this is a design question, not a bug, and I need your feedback.

The reason that I do not believe this is a bug, but rather this is functioning as intended, from AGX's side of things.

Here is the logic I am using for this:

Base KSP, neither kOS or AGX installed. There is no feedback on screen for action group status, but the way things are programmed action groups still activate/deactivate when their key is pressed, just nothing happens if there are no actions assigned to that action group.

KSP with kOS only, no AGX. There is still no feedback on screen for action group status, but needing a way to interact with a running program an empty action group makes perfect sense. If there are no actions assigned to that group, you can activate/deactivate it with impunity as it has no gameplay effect and provides a trigger that kOS can monitor while running.

KSP with AGX only, no kOS. There is now feedback on screen for all action groups. However, only action groups with actions assigned show up as it makes no sense to show empty action groups. Therefore to avoid confusion where the player can activate an action group that is not shown on screen, if a key assigned to empty action group is pressed, it is ignored.

KSP with both kOS and AGX installed. As far as activating action groups with actions assigned, there is no issue. The issue arises when the player desires to use an empty action group to server as a trigger for kOS code as they have previously with only kOS installed. In trying to do this the player runs afoul of AGX's ignoring of empty action groups as an empty action group has no feedback on-screen.

As a workaround, the "Script Trigger" action is added to all kOS computer parts. This is a null action that does nothing but servers as an action that can be assigned to an action group so AGX will show the action group on screen and allow activating/deactivating of that group. As a bonus, this allows the naming of said action group so the player can remember what they have set that group up to trigger in their kOS script.

In the original implementation of kOS working with AGX, I believed the Script Trigger was an accepted solution to this issue but it looks like it passed under the radar and the issue is now being raised.

So, the question is how acceptable is this from kOS's side of things.

As things stand, everything is fine from AGX's side, but you guys are the kOS devs so what are your current thoughts?

D.

Dunbaratu commented 9 years ago

I had never heard of ScriptTrigger before, but looking at the docs it does seem it was part of what you hashed out previously with other kOS devs who weren't me, and the docs do repeatedly say that only action groups 11 and up have the problem of needing it.

So I'm going to throw this back onto us kOS devs - who changed it so that action groups 1 through 10 no longer operate the stock way, and why? Apparently it was that which caused the problem, and that does deviate from what the docs claim.

The reason I never heard of ScriptTrigger is specifically because You used to not have to know about it for AG1 through AG10, so I never had to look up the AGX integration information yet. I only had AGX installed to ensure compatibility with how a lot of other people were running things, and to make sure if something about it broke I'd find out about it (which I just did with this issue). I hadn't had the need to use more than 10 action groups yet so I hadn't gotten in to AG11 and up, nor looked into the docs for how to do it.

So that again brings up, for the other kOS devs - why was this changed, and can we change it back? Can we make AG1 through AG10 work via stock again?

SirDiazo commented 9 years ago

Okay, I don't know what is going on here. We seem to be talking past each other and looking at your last post, the AGX documentation may need tweaking to clarify what is going on.

First, to double check what is being tested here. Using kOS and the "print ag1." command, you are checking to see if the state of the group ag1 is changing from False to True when you press the Alpha1 keyboard key which is bound to Action Group 1 when there are no actions assigned to Action Group 1.

As I am now at my computer and can test, I checked both the current versions (kOS 16.2 and AGX 1.31) and the original release versions (kOS 15.6 and AGX 1.29d) and on both of those setups this behavior is seen in that pressing the Alpha1 key does not change the state of "print ag1." at all and "print ag1." stays false. All tests with no actions assigned to action group 1.

This is as I would expect. To avoid conflicts, AGX locks out the default KSP handling of action groups so without an action assigned, AGX never activates the group and also blocks KSP from activating the group.

Where the AGX documentation looks to need clarification is that when no actions are assigned to that group, "ag1 on" and "ag1 off" behave as expected and will change the return of "print ag1" back and forth between True and False. This is what the documentation actually means by groups 1 through 10 behave the same while ag11 through ag250 do not.

The documentation needs clarification that you can not activate an empty action group in AGX directly, there is no button to click on screen and pressing the associated keyboard key will do nothing.

Now, is this accepted to you (the kOS devs), or is it a big enough deal that I should change AGX so that pressing a key bound to an empty action group toggles that group? Based on conversation at the time, I had considered the situation acceptable and moved on, but as it has come up again I'm willing to revisit it.

This entire post is talking about action groups with no actions assigned, if the group has an action assigned, everything behaves as expected.

Dunbaratu commented 9 years ago

Then the documentation definitely needs fixing. The fact that the keypress fails to change the action group state unless you've assigned some action to it in the VAB was not mentioned anywhere I could see, and it's a big deal for how people have been using kOS, and its not how it behaves in stock without AGX.

At the very least, we need to mention this fact not ONLY on the AGX special page, but everywhere the docs make the claim that you can read the state of "unused" action groups AG1..AG10 thusly. They need a caveat about how AGX effectively prevents KSP from changing the state of them via the keyboard, and a link to the AGX detailed page.

SirDiazo commented 9 years ago

Alright. As it appears this is a bigger deal then I thought it was and the Script Trigger action is not sufficient, I've figured out a hack where I can trigger the true/false state change on groups 1 through 10 so they behave as is intended by this issue.

There is no chance of groups 11+ getting this as the hack I'm using relies on the base 10 groups in stock KSP, but it will mean that groups 1 through 10 will behave the same regardless of whether AGX is installed or not.

I should have something for you guys to test tomorrow morning. I have the code finished but I won't have time to complete my tests and bug squashing tonight to get something I'm willing to release.

Dunbaratu commented 9 years ago

(By the way, thanks for the time you put into this issue.)

I think making it work only on AG1 through AG10 is okay. If it works differently for AG11 and up that's acceptable because that doesn't constitute altering how the stock game works. I'd think users would expect AGX's new custom groups it added to be different and require a slightly different mindset.

SirDiazo commented 9 years ago

Okay, can you try AGX 1.31a please?

https://github.com/SirDiazo/AGExt/releases/tag/1.31a

I think that will have the expected behavior to match stock (on groups 1-10) and that should resolve this issue.

If everything looks good I'll give the AGX doc page a look over as well.

erendrake commented 9 years ago

@SirDiazo i finally was able to test this issue with 1.31b and i was unable to reproduce with 1-10, ill try the upper action groups later but for now i think this is working again.