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

interface to show/hide gui. Returns previous state #20

Merged
merged 2 commits into from
Oct 27, 2015

Conversation

Choumiko
Copy link
Contributor

This allows other mods that add a (temporary) gui on the left to minimize YARM when their gui gets shown, when it gets closed they can restore YARMs previous state.
http://i.imgur.com/6sDjIOk.gif
Really only saves the player a few clicks, but i find it quite convenient.

@narc0tiq
Copy link
Owner

Okay, I like it, but two things come to mind:

  • first, I'm not sure how I feel about passing a complete event structure (from an unknown event) through to the remote interface. Using it from the console would involve something like /c remote.call("YARM", "hide_expando", {player_index = game.local_player.index}); on the other hand, this really isn't intended to be called from the console. That could be okay, then.
  • second, even when reduced, if you have a field at <= 10% of its original load, it will remain visible. Since the goal here is to help FARL's GUI always end up in the same spot (which I totally love!), there will be cases where that doesn't happen. Fixing it right, however, will probably require Better feedback via alerts/icons/tooltips #15; my current thought is that for sites that are within an error condition (e.g., site depleted, site has less than 10 minutes remaining, etc.), one would get a custom filter button in the vertical flow on the left. This way, the current expando can have the semantic of "hide all the sites, period".

Neither of these are blockers -- I'm okay with the patch as it is -- but I think I'd prefer if you matched the existing interface call, e.g.:

function interface.close_expando(player_name_or_index)
    local player = game.get_player(player_name_or_index)

    if global.player_data[player.index].expandoed then
        -- ...stuff...

I know, it feels wasteful to do a game.get_player call like that, but I think it helps both readability and robustness.

Alternatively, have it take a player object directly, as in function interface.close_expando(player) -- used from the console as /c remote.call("YARM", "hide_expando", game.local_player). The caller can be expected to have a player on hand, right?

@Choumiko
Copy link
Contributor Author

Didn't know that hiding doesn't work on low fields. I just got annoyed at the FARL GUI being in the middle of the screen while testing FARL (meaning jumping in and out of it a lot).

I'll go with matching the existing interface, player_index is really the only thing that's needed i guess. game.get_player might be wasteful, but it's gui code so that's ok for me. I'll add support for hiding YARM into my other mods too and probably to FAT Controller also (though in my current save it always opens at the leftmost side).
If the caller has a player it also has the index via player.index, so it's really no issue one way or the other.

narc0tiq added a commit that referenced this pull request Oct 27, 2015
Remote interface to toggle the expando. Returns previous state
@narc0tiq narc0tiq merged commit b4e72c1 into narc0tiq:master Oct 27, 2015
@narc0tiq
Copy link
Owner

Looks good to me, thank you.

@Choumiko
Copy link
Contributor Author

Choumiko commented Nov 7, 2015

I think this will need a change: if more than 1 other mod attempts to hide YARM you can run into a issue. Imagine YARM is maximized, no other mod gui is opened:

  • Mod A gets opened, requests YARM to minimize, recieves true as the previous state
  • Mods B .. E get opened, they too request YARM to minimize, they all recieve false
  • Mod A gets closed, requesting YARM to show up again.
  • Result: YARM is open while other mod gui's still want it to be minimized

A solution might be:

  • YARM keeps track how often it was requested to show/hide the ui
  • Only if only one mod wants YARM to be minimized and requests YARM to show again it returns the actual previous state (the one the first mod got returned), anything before that get's false as the previous state

This should fix the (theoretical) problem (I don't think there are any mods out there that make use of the new api yet)
I'll see if i can find some time tomorrow to add the api to my mods and test it a bit first before making a pull request

@narc0tiq
Copy link
Owner

narc0tiq commented Nov 8, 2015

It's gonna be fragile pretty much regardless of how you do it -- the biggest problem being that you're relying on the mods to always call "unhide YARM" when they're done.

How about we don't do any refcounting shenanigans and just rely on the player to unhide YARM whenever they feel like it? Another mod can request YARM to go into a specific state, but there's no compulsion to return it to the previous state -- something that the player might mess with regardless.

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.

2 participants