octgn / OCTGN

Online Card and Tabletop Gaming Network
http://www.octgn.net
Other
357 stars 129 forks source link

python functions that reveal hidden cards need to delay until the card is identified #416

Closed brine closed 9 years ago

brine commented 12 years ago

This would affect moveTo(), moveToTable(), isFaceUp(), maybe a few more I can't think of right now.

Basically, as mentioned in #412, the following python thinger occurs:

card.makeHiddenCardVisible (moveTo, isFaceUp, etc)
text = card.customProperty
notify("the card's property is {}".format(text))
>>> The card's property is ?

OCTGN needs time to identify the card and load its data from the database, but python doesn't care and just blitzes through the function. It tries to read customProperty but can't because the card's still loading

kellyelton commented: Ah...interesting...some functions should probubly block until they are complete, which would fix some of these issues then.

kellyelton commented 11 years ago

This problem is way more goofy then I originally thought. Reason it doesn't block is because a lot of functions just spit out network data, and then fire of the result whenever some network data comes back. So they are mostly async and can't really block. Might need to add some alternate versions of these functions with callbacks.

brine commented 11 years ago

this is a biggie still

BenMatteson commented 10 years ago

if this isn't closed by the addition of update() (i.e. we want to eliminate the need for update() altogether) it'd be nice to know the calls that need to be addressed.

BenMatteson commented 10 years ago

also, all the calls I've tested seem to work w/o any need for update (or rnd).

db0 commented 10 years ago

I don't know if anything has changed but not putting a delay is a cause for frequent issues, which are usually not even visible in solo testing.

I've even been running into issues with update() lately as it's not fast enough sometimes when a card changes piles.

BenMatteson commented 10 years ago

we've been doing some things to make it more predictable, but your right, I should be testing over the server and not just local. can you give any specific cases where you've been having trouble?

db0 commented 10 years ago

With update() or with card revealing? In the latter the classic one is flipping a card face-up and immediately trying to read its properties. Or moving a card from a non-visible group to a visible group and trying to read its properties. Haven't tested those lately since I've had my rnds in for a long time now and didn't see the need to remove them until now.

db0 commented 10 years ago

Also note that sometimes even my rnd is not slow enough, usually in the case where one of the player is lagging.

BenMatteson commented 10 years ago

I think that's not an issue anymore as of about the time reconnects were introduced, unless you've received reports since then. but I was actually a bit more curious about what you were talking about with update().

db0 commented 10 years ago

Hm ok, I'll try to test it and remove my rnds.

with update() I have this claimCard() function which I use to safely change controller of a card. I've noticed that without the loop, I was getting the error message you see there quite often. I then added the while loop which helped but I still see it when the card is in foreign pile (which would force the card to come to one of my piles as a means of changing control)

BenMatteson commented 10 years ago

ah, sorry, that's not the intended use for update(). what update literally does is stop script execution until any other functions in other threads (locally) have completed and resumes the script. the actual time delay is generally pretty short, especially when called repeatedly. as I understand it, update is intended for basically the use in this issue, where you reveal a card but need to wait for it to finish loading internally or you run into issues (though it doesn't seem to be needed here anymore anyway). to delay until something pings back from the other player like that requires rnd() (that's literally where the delay from rnd() comes from) or something. I feel like there should be a better solution for managing control, but that belongs in a different issue.

db0 commented 10 years ago

Ah ok, fair enough. Thanks for the clarification

BenMatteson commented 10 years ago

sure, sorry for the confusion, there were a lot of ideas on how update would work bouncing around before it was added, including replacing rnd(), but the consensus was that we can and should eliminate the need for the rnd() hack altogether, and just renaming it was pointless. might try using it to allow the interface to respond if you have a long running script, I'm not sure it's good for much else currently.

BenMatteson commented 10 years ago

though I think the threading of python was changed not too long ago as well, so that might be unnecessary now too... :stuck_out_tongue_closed_eyes:

db0 commented 10 years ago

I'm not sure it's good for much else currently.

Well, you can roll dice with it and I do have a few other tricks I'm using random numbers for ;)

BenMatteson commented 10 years ago

oh, I mean't update(). :P I know rnd() get's plenty of use besides delaying functions, and like I said, you still need it for taking control that way.

On Mon, Dec 23, 2013 at 2:47 PM, Divided by Zer0 notifications@github.comwrote:

I'm not sure it's good for much else currently.

Well, you can roll dice with it ;)

— Reply to this email directly or view it on GitHubhttps://github.com/kellyelton/OCTGN/issues/416#issuecomment-31148581 .

db0 commented 10 years ago

Taking control is being more difficult than expected. Whatever I do, I cannot wait enough until the card changes control. It will always fail. I've tried with both a 40-loop of rnd(1,10000) and also with time.sleep(1). It just never sees the card changing controller in the same execution where it sends the remoteCall to the current controller to pass control. it looks like changing control is stopped until all current executions end.