#gemrb@irc.freenode.net logs for 16 Jan 2012 (GMT)

Archive Today Yesterday Tomorrow
GemRB homepage


[00:00:25] <brad_a> oh i guess that +1 will increment it. i missed that
[00:01:16] <brad_a> oh i think i see what its for now :)
[00:01:27] <brad_a> those cursors change every frame
[00:01:37] <brad_a> some type of pulsing effect
[00:30:26] --> joneirik has joined #gemrb
[01:07:57] --> kamui has joined #gemrb
[02:44:31] <brad_a> fuzzie/tomprince: how does this look? http://dl.dropbox.com/u/13866402/cursors.patch
[02:45:50] <brad_a> still testing it myself
[02:50:19] <tomprince> brad_a: I would use release, instead of FreeSprite in the first patch, since we "know" the refcount is >1 at that point.
[02:50:28] <brad_a> sure
[02:52:17] <tomprince> Beyond that, I don't know. I have never looked at that code at all closel, os I don't know if it makes sense.
[02:53:27] <brad_a> ill see what fuzzie says tomorrow
[02:53:55] <tomprince> And, in any case, we should really add some doxygen comments, stating the refcounting semantics of these functions.
[03:05:59] <brad_a> good idea
[04:34:01] <-- brad_a has left IRC (Quit: brad_a)
[05:01:19] <-- gembot has left IRC (Quit: buildmaster reconfigured: bot disconnecting)
[05:01:33] --> gembot has joined #gemrb
[05:07:00] <-- gembot has left IRC (Quit: buildmaster reconfigured: bot disconnecting)
[05:07:13] --> gembot has joined #gemrb
[07:37:36] <-- demitar has left IRC (Ping timeout: 240 seconds)
[07:38:13] <-- joneirik has left IRC (Remote host closed the connection)
[07:47:03] --> Baldurer has joined #gemrb
[07:49:46] <fuzzie> so it turns out I wrote that scrolling code :-/
[07:51:39] <fuzzie> but I guess the mousescrollspd thing is from zefklop and so that part of the patch is ok
[07:52:02] <fuzzie> i am not convinced about the last chunk of the patch though?
[07:53:19] <fuzzie> i mean clearly it's very ridiculous as-is
[07:53:51] <fuzzie> but if it's going to be fixed, it should be in a separate patch which also e.g. removes the 'this is used only for the console' comment above the GetCursorSprite() function
[07:54:02] <fuzzie> please.
[07:54:05] <fuzzie> and now, coffee.
[08:24:30] --> edheldil has joined #gemrb
[08:24:31] --- ChanServ gives channel operator status to edheldil
[08:37:07] <-- nutron has left IRC (Changing host)
[08:37:07] --> nutron has joined #gemrb
[09:19:50] <-- Baldurer has left IRC (Ping timeout: 248 seconds)
[10:07:41] <-- edheldil has left IRC (*.net *.split)
[10:08:56] --> edheldil has joined #gemrb
[10:28:58] --> kettuz has joined #gemrb
[10:39:06] --> demitar has joined #gemrb
[10:50:16] --> SiENcE has joined #gemrb
[11:03:16] <-- demitar has left IRC (Ping timeout: 240 seconds)
[11:14:45] --> Baldurer has joined #gemrb
[12:40:59] <-- Baldurer has left IRC (Ping timeout: 276 seconds)
[13:38:07] --> demitar has joined #gemrb
[13:42:28] --> Baldurer has joined #gemrb
[13:52:30] --> lynxlynxlynx has joined #gemrb
[13:52:34] <-- lynxlynxlynx has left IRC (Changing host)
[13:52:34] --> lynxlynxlynx has joined #gemrb
[13:52:34] --- ChanServ gives channel operator status to lynxlynxlynx
[13:52:56] <lynxlynxlynx> back :)
[13:53:10] <lynxlynxlynx> nice progress in the meanwhile
[13:53:43] <fuzzie> hey lynx
[13:53:54] <fuzzie> went well?
[14:08:51] <lynxlynxlynx> excellent
[14:09:13] <lynxlynxlynx> too little sleep, but the conference was great
[14:09:59] <lynxlynxlynx> different than usual, much more empatic
[14:38:32] <edheldil> what conference was that?
[14:43:53] <lynxlynxlynx> http://www.letsdoitworld.org/conference
[15:51:54] --> brad_a has joined #gemrb
[15:52:54] <brad_a> fuzzie: are you by chance confusing GetScrollCursorSprite with GetCursorSprite?
[15:57:12] <fuzzie> quite possibly
[15:57:39] <fuzzie> the 'separate patch' bit is because it seemed to be a completely different SetCursor call to the video one
[15:57:43] <fuzzie> is it not?
[15:58:27] <brad_a> no it the latest version of the patch it is simply: core->GetVideoDriver()->SetCursor(cursor, VID_CUR_DRAG);
[16:00:42] <fuzzie> i am really confused actually
[16:00:44] <fuzzie> which patch?
[16:00:56] <brad_a> also about that patch your reverted yesterday. isnt the "dest->XPos = dest->XPos;" line wrong and a problem?
[16:01:02] <brad_a> hold on ill post a link
[16:01:09] <fuzzie> yes, but removing the line is incorrect
[16:01:21] <fuzzie> and removing the other stuff was completely crazy
[16:01:31] <fuzzie> Avenger applied a correct patch after discussing on IRC
[16:01:41] <brad_a> sure because duplicatesprite doesnt duplicate the x/y
[16:01:54] <brad_a> as we thought i guess
[16:02:13] <brad_a> oh i didnt see that part
[16:02:30] <brad_a> well then that is good i just thought you were unaware of that!
[16:03:11] <brad_a> http://dl.dropbox.com/u/13866402/cursors.patch
[16:03:24] <fuzzie> right, that's the one I'm looking at
[16:03:27] <fuzzie> what's with the last chunk?
[16:03:27] <brad_a> wait thats the old one
[16:03:32] <fuzzie> ah ok :p
[16:03:50] <brad_a> its mostly the same tho
[16:04:48] <brad_a> ok same link
[16:05:11] <gembot> build #19 of autotools g++-4.4 is complete: Success [build successful] Build details are at http://buildbot.gemrb.org/builders/autotools%20g%2B%2B-4.4/builds/19
[16:05:48] <brad_a> the only dif is the consolidation of getvideo and set cursor plus changing freesprite to release
[16:06:24] <fuzzie> right, so the last chunk still doesn't make any sense to me
[16:06:57] <brad_a> you mean the 2nd commit?
[16:07:02] <fuzzie> the last part of the 2nd commit
[16:07:34] <brad_a> well getCursorSprite returns a new sprite correct?
[16:07:40] <fuzzie> yes
[16:07:51] <fuzzie> and edit->SetCursor takes ownership of it
[16:08:04] <brad_a> yes so then after that i release it
[16:08:05] <fuzzie> so if you call release(), you just threw away the last reference
[16:08:10] <brad_a> ?
[16:08:12] <fuzzie> and it gets deleted from underneath edit
[16:08:18] <brad_a> doesnt setcursor do an incref?
[16:08:22] <fuzzie> nope
[16:08:53] <brad_a> ah cuz edit isnt what i had assumed it was
[16:08:55] <brad_a> :-P
[16:09:05] <fuzzie> 17:01 <fuzzie> the 'separate patch' bit is because it seemed to be a completely different SetCursor call to the video one
[16:09:09] <fuzzie> :-)
[16:09:35] <fuzzie> i mean, maybe you want to add an incref there and still do it this way, but it seems not-good as-is
[16:09:38] <brad_a> sorry i thought you were talking about part 1 cuz you said something about the console cursor i dont know ists super early here :-P
[16:09:41] <fuzzie> hehe
[16:09:55] <brad_a> i havent yet had my coffee
[16:10:00] <fuzzie> well as usual we are not in a rush
[16:10:12] <brad_a> well this is why i ask for your input!
[16:13:26] <brad_a> its going to be a long day. i just tried to do git rebase --amend… sat here for a bit scratching my head.
[16:13:40] <brad_a> also so much for snow in the forecast...
[16:16:46] <brad_a> ok fuzzie same link. took out that bit
[16:21:29] <fuzzie> well it looks fine at a glance
[16:21:32] <fuzzie> but do test it :-p
[16:25:19] <tomprince> But 'if (DraggedCursor) DraggedCursor->release();' seems like a complicated interface ....
[16:25:27] <brad_a> yes
[16:26:07] <fuzzie> well, I am not too worried if we only actually end up doing it in two places
[16:26:25] <fuzzie> especially since gemrb will complain about leaked sprites
[16:26:41] <fuzzie> but not particularly dedicated to one solution or the other as you can tell :)
[16:27:30] <brad_a> i know i prefer the always release what you create and take ownership of what you want to keep around and release when done model :-P
[16:27:44] <brad_a> but that may be because thats how cocoa works....
[16:28:32] <fuzzie> well, it's the only model which doesn't eventually make someone's head explode, I think
[16:28:49] <brad_a> hehe
[16:29:43] <-- gembot has left IRC (Ping timeout: 248 seconds)
[16:30:01] <brad_a> im still not too sure about the portrait drag cursors...
[16:30:37] <brad_a> i think there is somewhere doing a release that i need to remove now
[16:33:26] <fuzzie> I haven't looked, admittedly.
[16:33:37] <fuzzie> Will do so after cooking dinner, but bit inconvenient from kitchen.
[16:34:55] <brad_a> yes. like you say no hurry :)
[16:35:08] <brad_a> it may have behaved this way before for all i know
[16:35:44] <brad_a> but when dragging portraits the cursor is reset after the portrait swap even if you are still dragging
[16:39:29] <brad_a> also discovered a bug. hitting attack then stop results in "no breakable action" error.
[16:39:47] <brad_a> maybe im wrong tho
[16:40:00] <brad_a> seems like stop should clear whatever pending action
[16:40:29] <-- SiENcE has left IRC (Quit: @all: cya)
[16:52:07] <brad_a> the culprit appears to be the line at the bottome of OnGlobalMouseOver
[16:52:35] <brad_a> i thik it ought to be removed entirely but im not sure
[16:53:51] <brad_a> i guess that line clears the scroll cursors tho
[16:54:37] <brad_a> mybe updatescrolling can do that...
[17:02:56] --> Maighstir has joined #gemrb
[17:28:43] <-- brad_a has left IRC (Quit: brad_a)
[17:36:32] --> gembot has joined #gemrb
[17:45:01] <-- DrMcCoy has left IRC (Ping timeout: 252 seconds)
[17:48:48] --> brad_a has joined #gemrb
[17:49:24] --> DrMcCoy has joined #gemrb
[18:26:28] <tomprince> (anyway, ignore my comment, since I was confused and then lost my connection)
[18:37:22] <brad_a> comment?
[18:38:18] <tomprince> My comment about complicated interface. I was confused, realized my mistake a couple of minutes latter, but was disocnnected by that point.
[18:38:30] <brad_a> ah
[19:05:49] --> Yoshimo has joined #gemrb
[19:09:05] <-- DrMcCoy has left IRC (Quit: He's dead, Jim)
[19:09:33] --> DrMcCoy has joined #gemrb
[19:26:35] <-- Baldurer has left IRC (Ping timeout: 276 seconds)
[19:31:06] --> SiENcE has joined #gemrb
[19:54:36] <-- demitar has left IRC (Ping timeout: 240 seconds)
[19:56:46] --> demitar has joined #gemrb
[19:57:03] <-- harijan_ has left IRC (*.net *.split)
[19:57:14] --> harijan has joined #gemrb
[20:09:16] <-- demitar has left IRC (Ping timeout: 240 seconds)
[20:40:53] <brad_a> it doesnt exactly seem like the RightWay™ but the easiest way to fix drag cursors seems to be replace line 1538 in game control with this: if (!core->GetDraggedPortrait()) core->GetVideoDriver()->SetCursor(NULL, VID_CUR_DRAG);
[20:41:20] <brad_a> are there any other "true" drag cursors tho?
[20:43:11] --> Baldurer has joined #gemrb
[20:43:31] <brad_a> if there are maybe the best thing to do would be replace dragortrait with a more universal name and set it for all drag opperations?
[20:46:29] <brad_a> except i see now that dragportrait is nothing more than an index
[20:48:53] <brad_a> i cant really think of any other draggable things… so maybe it is an acceptable fix?
[21:02:45] <fuzzie> you mean other than what you patched?
[21:03:58] <brad_a> yes in addition to that patch
[21:04:08] <brad_a> i think this was broken before
[21:04:25] <fuzzie> i mean, just that single line clearly isn't an acceptable fix
[21:04:40] <brad_a> tho i havent decided if im too lazy to checkout/rebase to before and test to see if it was broken before
[21:07:03] <fuzzie> but I don't see how drag cursors work at all with that line, very confused
[21:08:03] <brad_a> well the problem is that once you leave the bounds of the portrait you are attempting to drag then setdraggedportrait ceaces to be called so the globalmousemove method does get called and clears the cursor
[21:08:19] <lynxlynxlynx> the only problem i can remember from before was if you ended the drag outside the window
[21:08:21] <brad_a> im gonna roll back before my changes to see if it was a problem then
[21:08:32] <fuzzie> yes, but we implement e.g. containers with the drag cursors
[21:08:42] <fuzzie> so resetting them anywhere seems a Very Bad Idea
[21:09:08] <brad_a> if you meean when you pick up an item that still works
[21:09:14] <fuzzie> oh right, gamecontrol is hidden in inventory etc
[21:09:29] <brad_a> the only problem are with things where an actual drag is involved
[21:09:38] <fuzzie> you think?
[21:10:01] <brad_a> the only thing i know of but im not sure im testing everything
[21:10:33] <fuzzie> it seems more likely we're just sort of coincidentally mostly avoiding the problem because the gamecontrol is hidden in most places you'd notice
[21:11:08] <fuzzie> certainly I was testing the portraits in other screens :-p
[21:14:30] <brad_a> well i just checked out to before i made any cursor changes and it is indeed broken. i can clean build to make sure tho
[21:15:16] <fuzzie> it doesn't seem unlikely
[21:18:14] <fuzzie> a quick look at git history blames this on "touchscreen update from Beholder (with small mods)" by Avenger
[21:18:18] <fuzzie> so I don't know which of them is responsible
[21:19:23] <fuzzie> previously (dating back beyond the 3-year-old movement of this code here from SDLVideo), the drag cursor change was wrapped safely after the 'scrolling = false;' bit
[21:22:33] <fuzzie> and so my ill-thought-out proposal is to put that back the way it was although I don't see how on earth I keep the cursor there
[21:26:41] <fuzzie> the python-side CheckDragging is not very nice. but moving the line back to where it was helps.
[21:28:55] <brad_a> sorry i got wayliad
[21:30:22] <fuzzie> I am concerned you might not be taking the "no hurry" thing seriously. :)
[21:30:41] <brad_a> hehe maybe
[21:31:40] <brad_a> i do know without the globalmousemove clearing the drag cursor then when you scroll the viewport with the mouse those cursors get stuck until yoou do something else
[21:32:07] <brad_a> but with that the drag cursor for portraits and who knows what else gets cleared when it shouldnt
[21:33:10] <fuzzie> well, it seems to have been randomly moved around in that touchscreen patch and I don't think that's good
[21:33:21] <fuzzie> should be fixed properly if it's an issue
[21:33:32] <fuzzie> but just random opinion
[21:33:48] <brad_a> so i guess there was a time hen it did in fact work?
[21:34:39] <fuzzie> well, as I said, it used to be after the 'scrolling = false;' bit
[21:35:12] <fuzzie> but that might well cause problems if you're scrolling and then you switch to a textscreen or another screen
[21:35:58] <fuzzie> apparently I wrote quite a bit of this code but I've long-since forgotten it and I don't recognise anything so..
[21:44:52] <-- Yoshimo has left IRC (Quit: Yoshimo)
[21:45:18] <brad_a> well if it makes you feel any better i seem to stop recognizing code in a matter of weeks not years :)
[21:46:06] <brad_a> ok lets revert that line to agter the scrolling = false
[21:46:08] <brad_a> that fixes it
[21:46:39] <brad_a> ok so can i push that work now since apparently i didnt break that?
[21:46:40] <fuzzie> if you commit it, do comment it
[21:46:58] <brad_a> comment the code?
[21:47:00] <fuzzie> but the leak changes seem fine to me (at a glance)
[21:47:06] <fuzzie> comment the drag cursor reset there, i mean
[21:47:35] <brad_a> you mean in code or in the commit message? because i was just going to do a new commit for that....
[21:48:11] <fuzzie> in both ideally :-p but mostly the code
[21:48:19] <fuzzie> since it got moved once already..
[21:48:35] <brad_a> is there a better way to throw out the last commit other than rebase -i and delete the line?
[21:49:14] <fuzzie> well I'd just do 'git reset --hard HEAD^' but honestly that's probably a bad idea if you're not comfortable enough to do it yourself :-p
[21:49:33] <brad_a> thats a bad idea because that would kill things i want to keep ;-)
[21:49:35] <fuzzie> also destroys working tree changes but I assume if you're rebasing you already dealt with that
[21:51:51] <brad_a> wait i guess HEAD is just the latest commit not what origin/master is...
[21:55:04] <-- DrMcCoy has left IRC (Read error: Connection reset by peer)
[21:55:48] <fuzzie> yes
[21:58:52] <CIA-36> GemRB: 03bradallred * r52858ae32eb5 10gemrb/gemrb/core/Interface.cpp: Cursors: fix leaking drag cursors.
[21:58:53] <CIA-36> GemRB: 03bradallred * r9b5a7145c044 10gemrb/gemrb/core/GUI/GameControl.cpp: GameControl: rewrite OnGlobalMouseMove to play nice with the cursor refactor.
[21:58:53] <CIA-36> GemRB: 03bradallred * r10cda8b5cb75 10gemrb/gemrb/core/GUI/GameControl.cpp: GameControl: don't clear the drag cursor on mouse move except when "scrolling" ceases to be true. Make a note of this so it stops getting moved.
[22:01:51] <brad_a> if you take out the comments that has a negative loc balance :D
[22:12:31] <-- SiENcE has left IRC (Quit: cya)
[22:26:54] <brad_a> im a bit of a dope tho. i put globalmousemovve in that message instead of updateScrolling...
[22:27:19] <brad_a> can i fix that in a non-hairpulling mannor?
[22:27:41] <brad_a> now that i have pushed i mean
[22:45:49] <lynxlynxlynx> nope
[22:46:26] <brad_a> i somehow lost that git flow chart you posted :)
[22:46:45] <lynxlynxlynx> rewritting public history is bad
[22:46:59] <brad_a> yeah i figured
[22:47:15] <brad_a> i was hoping rewriting just the commit message wasnt tho
[23:02:02] --> joneirik has joined #gemrb
[23:04:20] <-- Baldurer has left IRC (Ping timeout: 276 seconds)
[23:18:25] <CIA-36> GemRB: 03avenger_teambg * r58f6d3a34c47 10gemrb/gemrb/plugins/IWDOpcodes/IWDOpcodes.cpp: better lower resistance opcode for iwd2
[23:30:38] <brad_a> would the correct way for me to implement autopause for when gemrb is suspended to set a config var then bitwise or with "Auto Pause State" with vars->setat
[23:31:21] <brad_a> or not even bother with the config var and just always set AP_GENERIC flag on the dictionary value?
[23:31:53] <brad_a> seems like an unnecissary configuration to me.
[23:34:15] <brad_a> im thinking i may as well just do autopause_flags |= AP_GENERIC after looking up the value or doing im thinking vars->SetAt("Auto Pause State", vars->Lookup("Auto Pause State") | AP_GENERIC) during init
[23:35:01] <tomprince> I've always disliked vars ...
[23:35:49] <brad_a> well im open to whatever :)
[23:37:21] <brad_a> what is happening now is SDL_WINDOWEVENT_MINIMIZED triggers core->Autopause(AP_GENERIC, NULL); so that the game is paused when you return to it after taking a phone call or whatever
[23:37:57] <brad_a> i have uncommited code in interface to actually make it happen but i need to merge it in the correct way
[23:39:34] --> DrMcCoy has joined #gemrb
[23:40:45] <brad_a> im not sure how i feel about baulder.ini having precedence over gemrb.cfg btw
[23:46:29] --> brad_a_ has joined #gemrb
[23:47:55] <-- lynxlynxlynx has left IRC (Read error: Connection reset by peer)
[23:49:09] <-- brad_a has left IRC (Ping timeout: 244 seconds)
[23:50:59] <-- brad_a_ has left IRC (Ping timeout: 255 seconds)