Disable python functions to manipulate cards/piles you don't control

[brine]

There has always been an issue with OCTGN involving the manipulation of cards you own but don't control; Jods knew about it for years but never figured out a way to fix it. He mentions in the Python API wiki page:

"As of OCTGN 0.9, the Python scripts do not enforce controlship on the object they manipulate. It is not safe to change the state of cards or groups you don't control, even though the script would let you do that. The only functions that are safe to use on elements you don't control are target, highlight, counters and markers manipulations. Of course you can read the state of any element. So be very careful to always filter the cards you touch. DO NOT use (c for c in table); DO use (c for c in table if c.controller == me).

This issue can cause OCTGN to crash in the case where a python function tries to move a card on the table at the same time that another player is trying to manipulate that card (such as click-dragging it around the table). The "Scoop" function in MTG provides an example of where such a crash may occur, as it attempts to return cards you own but no longer control to your deck.

This whole issue is probably a consequence of the way OCTGN handles objects, so I can't imagine a simple fix (if a fix can even exist). However, I feel it's important to note that this issue exists in the event someone else runs into it

[ghost]

Maybe octgn needs something like user accounts ? Octgn itself would be the Administrator, or root (?), with players = users, which would get some kind of attributes, to read / write (here: move, touch cards).

[brine]

or a new attribute on the Card class, which sets to "Locked" whenever a player activates an OCTGN function on it. Such as dragging the card (leftclick held down), accessing the right-click menu, running a python function, etc. Then once that function is either completed or spontaneously terminated (if the python function catches an error, for example), it sets the attribute to Unlocked.

Then players cannot perform any sort of OCTGN function on that card until it contains the Unlocked attribute.

[ghost]

Could this be done with while loop ?

while function == active: move.card = True

[kellyelton]

The problem is the way that all the octgn action are invoked. It would be different if every action had an undo option, in which case we could bounce whatever we wanted off the server and the server could be like, "Whooo, hold up there. This guy is already doing something, return to your original state and try again". Currently whenever octgn does a move for example, it will physically move the card, and then send data, instead of waiting for the server to tell it anything. The reason for this is so that the program can feed responsive, regardless of any kinds of lag. This would be fine if we could undo the action that could have collided, but at this point there is no real way to do that.

Until we can have a generic action system, with each action having a do and undo, there is no way to know exactly how to undo actions, or groups of actions in tandum.

So, I propose that instead of trying to solve this issue with other non-conncurent means(letting the server tell you if it's ok, or the other player), we just fail python commands trying to manipulate cards that aren't controlled by you. This will lead to more stability because it's technically possible to crash the client otherwise.

[brine]

I haven't heard any issues about this come up lately, maybe its not as bad as it used to be

[kellyelton]

I guess more over I wouldn't call this a bug, I would call it a limitation in the engine and networking.

[kellyelton]

Well none of that really changed, I think some have gotten crushed because of random try catches in odd places, but this is still very much a limitation of the system.

But the random try catches don't actually fix anything, they actually end up breaking state, which just leads to a crash later on down the road.

[kellyelton]

I'm not saying it'll happen tomarrow, but it's something that should be implemented. The ability to do this breaks the entire state machine of the game.

[kellyelton]

I'm sure that this is one of the reasons random crashes happen. Over zealous users mashing hotkeys. The key concept here is that you don't control the object in question. That's a core piece of terminology in OCTGN, it means that the player, and by extension python, has no rights tinkering with any aspect of that object, except to regain control.

This is in place specifically to allow for gameplay where users can perform actions at the same time, since there is no chance of you physically moving my cards when I move them, save you do a gain control request.

If we change it, we would be required to change to a system that only allows for actions to be done by one player at a time, and lock down the turns like control over objects is locked down. Now with some networking overhaul, and a big refactoring a ways past 100%, this may change due to the server and most of the networking being rewritten, but at this time this function is necessary and will be implemented at some point, unless some form of rewrite happens first.

In the nicest possible way: If this happens it wouldn't be "for no good reason", and causing some games to have to rewrite some functionality is almost irrelevant. This is why you should stick to the outlined parameters, and not try and find exploits, cause as far as I'm concerned this is an exploits, not a feature being removed.

[brine]

just a brainstorm here:

What if the game def used the moveCard event trigger to control all manual manipulations of a card, and its set up to simulate the manual movements of cards? From what I remember, the old crashes happened when the opponent's python tried to move the card that you were currently click-dragging, but haven't yet dropped. Maybe if python managed all the movements, it wouldn't be as flaky

[kellyelton]

There isn't just one old crash, there are constant crashes, and new crashes. As soon as I patch one game play crash up, another springs up. It's hard to pin down the exact cause, and most likely people are seeing less errors not because what's being done is fine, but because I'm duct tapping the holes as they spring up.

[kellyelton]

For example, in the past 24 hours there have been 130 fatal errors, meaning octgn actually shuts down, and most of them are related to game play. Every few weeks or so I read through them, and patch up the errors. Good thing is that crash won't happen again, but what will most likely happen is a new crash will end up happening 10 steps down the line.

[brine]

I guess technically with the new events system, you can still effectively manipulate your opponents cards by sending them the event through global variables.

I'll use my card alignment code as an example. Alignment fires, all of my cards get moved around to their correct locations. Then it changes a globalVariable, which triggers the event for the opponent and their own alignment function fires, moving their cards around.

[kellyelton]

That would be the recommended way, because the opponent and the opponents script engine is actually doing the modification, so it can lock on itself and keep that user for breaking state.

[db0]

OK, so if it's possible to do the same thing (move opposing cards) via python and shared variables, why not simply improve the moveTo() and moveToTable() functions to have that failsafe functionality built-in? Why not set an internal shared variable when a card object is being manipulated by one player, and if another player tries to manipulate the same object at the same time and sees the shared variable set, wait for 1 second and retry again and cancel out after three attempts.

This is the way I would do it in python for example, so wouldn't it make sense to simply improve the robustness of the built-in APIs?

[kellyelton]

We're beefing up the logging that goes around games to include: game name, version, id, username, python details, and other various things, as well as a log python function for you guys to write to the octgn log itself.

As far as manipulating cards you don't control, that's not going to happen. If you want to control another persons controlled object, the best I can offer you is a call that allows you to call a python function on another persons client. Doing that would be good for sealing this exploit up.

[db0]

But that's functionally the same thing! You're just using a different function. All I'm saying is to simply keep the same function, and use that call internal to the existing APIs. So when I use moveTo() on my own cards, but when I use moveTo() on opposing cards you instead send the call to the other person's client.

[db0]

Also can you define: "manipulating cards"? Would this include adding markers, changing orientation, changing faceup state and changing alternate art?

[kellyelton]

anything that changes the state of the card. And it's better for you to make up your own python calls as opposed to letting octgn magically handle it. That obscures things too much, especially if there are errors and debugging going on. Better that the developer implicitly knows that he's making a network call.

[db0]

Why would it matter if I know it or not? From the perspective of the developer, all that should matter is that I want to manipulate a card.

[brine]

cardAlignment():
    {a bunch of code to rearrange all my-controlled cards on the table}
    setGlobalVariable('triggerAlignment', [playerB, playerC])

Then we set up a Global Variable change event for 'triggerAlignment', and the variable contains a list of all the players who need to get their cards aligned.

triggeredAlignment():
    for player in eval(getGlobalVariable('triggerAlignment')):
        if player == me:
            {a bunch of alignment code}

So basically, the alignment code will align everybody's cards, but it will distribute it among all the players so they'll only align their own cards.

Part of the problem with moving around your opponent's cards is that the server doesn't stack inputs in the same way that the local clients do. Cuz the server has to relay the changes between the players, and keep the gamestates consistent. When you're only dealing with your own controlled cards on your own local client, the inputs are queued. When you have two local clients fighting over the same game object, both communicating the changes to the server so they get relayed to the other players, unpredictable things happen. The local game states go out of sync.

[db0]

Shared variables are nice if all you want is something as simple as that, but when I do things like trying to grab properties of a hidden opposing card in the middle of one of my scripts (which includes changing the foreign card's state temporarily) before displaying information to the local player pertaining to that card, then things get very tricky.

[kellyelton]

Your ability to do that right now is a bug. The 'golden rule' as it was stated before is to not modify anything you don't control. I've given good reasons as to why that's important, and fromyour responses you don't refute the legitimacy of the concern. So what this argument boils down to is you don't like the idea of a new api in place to allow you to invoke methods on the remote client, like would be developed in good software, and would allow you to do a lot more than just forwarding existing ones transparently.

If the idea was that I'm taking away functionality here, then this wouldn't be happening(at some point who knows when), but the thing is, it's a bug. There are safeguards built in for a reason.

[db0]

I'm not opposed to you giving APIs for foreign manipulation. I'm saying there's no point in splitting card manipulation to local and foreign card APIs. I'm saying it makes no sense for a game developer to have to care about whether a card object is their or not as long as OCTGN handles the manipulation gracefully.

[kellyelton]

https://github.com/kellyelton/OCTGN/wiki/OCTGN%20python%20api%20reference#remotecallplayer-function-arguments

[db0]

Can the local player use this to run their own functions? If they do, do those functions fire during the current execution, or are they queued for afterwards, like events?

[kellyelton]

this doesn't block, the execution happens sometime in the future, and there is never a garantee that the result will be what you wanted.

[db0]

and there is never a garantee that the result will be what you wanted.

Wat?

[kellyelton]

if you say 'setCounterCountTo(12)' or something, and the card ends up in the users hand before that event fires, obviously the counter count won't be 12

[kellyelton]

Basically, you can't ever expect a call to work, you have to check afterwards to see if it did, just because in the time it takes to travel through the server then to the other users machine, something might have changed.

[kellyelton]

So in that light, I would say, no, don't fire this on yourself unless you are looking to fire something off kind of asynchronously, like you don't care about how long it takes or if it works...like downloading your scripts would be a perfect example of when to use it on yourself. Besides that though, moving cards, adding tokens, shuffling all would be bad ideas to do on yourself.

[db0]

Is there a possibility to create some tool to help with the code migration to work with this or am I on my own?

[kellyelton]

I think you're basically on your own. I'm not sure off the top of my head how I would make a tool for that.

[db0]

Basically I need to convert a lot of card class calls to function calls instead. Along with debugging, this is a very heavy rework for me alone, so I will need ample time to switch.

I see some "convertostring" stuff going on in your functions. Do variables passed to the other player's functions maintain their type?

[kellyelton]

Any kind of switch isn't going to happen any time soon. While I would start working on it, I wouldn't feel rushed if I was you.

As far as keeping their type, yes they do. The caveat is if you send a card that's visible to you, but not visible to the other person, they will only see their version of that card, without any card specific details. This goes for any object that can be obscured based on who you are.

If I was you I would just test the bounds of it. It should be able to transport markers, cards, players, groups, piles, hand, lists, strings, ints, and probably many more things.

[db0]

Thanks. Is this already live?

[kellyelton]

To go farther as far as not feeling rushed, the main reason I implemented this is because doing the whole globalvariable event to fire methods wasn't a very good solution, and I had already did some work adding click events for a game dev, but at the end of the day what they were doing would have required them to fire events remotely. I didn't spend time on this to change the way things work, it was to help a game dev accomplish a goal in a cleaner manor. It's just a consequence that it's also one of the first steps to completing this issue, but again I don't even think about this issue, and most likely won't plan for it until 100% feature funding or so.

[kellyelton]

Yeah it's live.

[db0]

Oh right. Thanks, that really takes the pressure off my mind then as I do get kinda panicky just thinking about it >_<