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

Archive Today Yesterday Tomorrow
GemRB homepage


[00:01:45] <tomprince> As I was going through, I noticed a few things, and wasn't sure how best to handle them.
[00:03:47] <tomprince> - We print a number of things with printStatus. Lots of ERROR and WARNING, but also NOT FOUND and SKIPPING (plus what resource manager does). I wasn't sure if we wanted to keep that variety, and whether we should pass strings everywhere, or use an enum for that.
[00:05:55] <tomprince> - The ERROR vs. WARNING split seems quite inconsistent. I tended to make things that didn't propagate an error into WARNINGS, and everything else into (non-fatal) ERROR.
[00:07:42] <tomprince> - I'd like to change Interface::Init to return void, and just call error on the 'return GEM_ERROR' paths.
[00:45:21] <-- brad_a has left IRC (Quit: brad_a)
[01:34:34] --> brad_a has joined #gemrb
[01:35:25] <brad_a> hopefully whoever implemented the YUV video playback is still around :-p
[01:35:59] <-- nutron has left IRC (Ping timeout: 245 seconds)
[01:42:53] <-- lostLinSoul has left IRC (Quit: Page closed)
[01:45:13] --> nutron has joined #gemrb
[01:56:24] <-- kettuz has left IRC (Quit: Leaving)
[02:21:15] <brad_a> wtf is GEM_GRAB?
[02:38:10] <brad_a> finally got SDL 2 working :)
[03:06:43] <brad_a> ok sdl 1.2 works again too :p
[04:41:13] <-- gembot has left IRC (Quit: buildmaster reconfigured: bot disconnecting)
[04:41:26] --> gembot has joined #gemrb
[04:49:50] <-- joneirik has left IRC (Remote host closed the connection)
[05:00:51] <brad_a> tomprince, fuzzie, et al: https://github.com/bradallred/gemrb/tree/SDL
[05:47:41] <-- brad_a has left IRC (Quit: brad_a)
[07:10:22] <Kiranos> http://www.gamebanshee.com/interstitial/interstitial.html?oldurl=http://www.gamebanshee.com/news/106895-the-infinity-engine-goes-open-source-and-cross-platform-sorta.html
[07:14:33] <Kiranos> http://twitter.com/#!/ChrisAvellone retweeted, pretty funny
[07:14:35] --> lynxlynxlynx has joined #gemrb
[07:14:36] <-- lynxlynxlynx has left IRC (Changing host)
[07:14:36] --> lynxlynxlynx has joined #gemrb
[07:14:36] --- ChanServ gives channel operator status to lynxlynxlynx
[07:21:57] <lynxlynxlynx> that's nice :)
[08:11:43] <lynxlynxlynx> tomprince: looks fine to me
[09:06:08] <-- Kiranos has left IRC (Quit: No Ping reply in 180 seconds.)
[09:07:14] --> Kiranos has joined #gemrb
[09:10:55] <edheldil> Kiranos: interesting - but is it possible to see what was that last guy asking? I do not use twitter :)
[09:12:22] <Kiranos> his tweets :) "A version of the Infinity Engine for all. Very nice! gemrb.org/wiki/doku.php?…"
[09:12:29] <Kiranos> "@wolfennights Although, the link I sent out doesn't require any fee at all, it's open source and fair game, which is incredible."
[09:12:46] <Kiranos> pretty cool coming from torment creator
[09:13:33] <Kiranos> @ChrisAvellone And you were able to get the licensing because you were their publisher? What inspired you to want to get licensing?
[09:13:41] <Kiranos> "@wolfennights Although, the link I sent out doesn't require any fee at all, it's open source and fair game, which is incredible."
[09:24:27] <-- kamui has left IRC (Read error: Connection reset by peer)
[09:31:11] <edheldil> Ah, you can just click the links :)
[09:31:25] * edheldil bows his head in shame
[09:31:30] <edheldil> Thanks
[09:51:12] --> SiENcE has joined #gemrb
[10:30:04] --> kettuz has joined #gemrb
[11:57:20] --> lostLinSoul has joined #gemrb
[12:07:32] <lostLinSoul> In Interface.cpp, there I code I do not uderstand - it start with if(false) { #define CONFIG_INT(str, var) \ } else if (stricmp(name, str) == 0) { \ var (atoi(value) ) ....etc.
[12:08:31] <lostLinSoul> Start with 'if (false)', I would never have expected it to run in the first place.
[12:10:18] <lostLinSoul> No idea where 'str' nor 'var' come from
[12:13:05] <lostLinSoul> What does var (atoi(value)) do exaclty?
[12:15:18] <lostLinSoul> Likewise CONFIG_INT("Bpp", Bpp = ); - I have missed anything that defines what CONFIG_INT actually does...
[12:17:41] <lynxlynxlynx> yes, if false never matches
[12:18:25] <lynxlynxlynx> but the macro is defined or it wouldn't work
[12:22:19] <lostLinSoul> It all in Interface::LoadConfig(const char* filename)
[12:25:17] <lostLinSoul> I did a grep of the entire directory structure and CONFIG_INT was only found there - I have found no other defineition other than if(false) { #define CONFIG_INT(str, var) \ }
[12:25:45] <lostLinSoul> I showed above
[12:27:49] <lynxlynxlynx> yes, but it is a macro
[12:28:01] <lynxlynxlynx> a substitution before the compiler gets the code
[12:29:29] <lynxlynxlynx> the compiler will see if(false) { CONFIG_INT("Bpp", Bpp = ); }
[12:29:34] <lynxlynxlynx> bbl
[12:58:36] <edheldil> lostLinSoul: the macro itself, CONFIG_INT compares key read from the config file (name) to a given string, and if it matches, assigns the key value coerced to into a given variable (e.g. Bpp)
[12:59:58] <edheldil> the "if (false)" allows all the macro -expanded conditions to be the same, i.e. "} else if ...."
[13:00:39] <edheldil> ... coerced to int into ...
[13:07:03] <lostLinSoul> edheldil: Thanks, it's a bit clearer - but I'm still getting lost on the syntax and understanding exactly what is happening
[13:07:25] <lostLinSoul> Is there sonewhere I can read about this form of usage?
[13:08:10] <edheldil> c/c++ manuals. It's really basic, although it looks a bit strange on the first sight
[13:09:01] <lostLinSoul> The braces seem to be mismatching...and I dosn't see how 'if (false)' allows what you said
[13:11:35] <-- kettuz has left IRC (Quit: Leaving)
[13:18:32] <lostLinSoul> Ah, I think I have it now
[13:22:34] <lostLinSoul> Dang that was confusing!
[13:45:38] <wjp> oh wow that is ugly
[13:48:12] <wjp> I can appreciate it in a "clever syntax" way, but, ugh
[13:50:07] <wjp> and all that to get rid of some stricmp lines
[13:51:24] <edheldil> hehe, I share this sentiment
[13:51:56] <DrMcCoy> Eh, that's nothing. I've once seen that construct:
[13:51:56] <DrMcCoy> #define foo if (something) {
[13:52:54] <DrMcCoy> Not that left-openend bracket there. That macro was a hell to read and keep track of when used in the code
[13:52:57] <DrMcCoy> Note*
[13:53:02] <wjp> :-)
[15:15:44] --> Yoshimo has joined #gemrb
[16:04:04] <lostLinSoul> Another curios piece of code is:
[16:04:09] <lostLinSoul> size_t cnt = CD[i].size();
[16:04:17] <lostLinSoul> while(cnt--) {
[16:04:29] <lostLinSoul> ResolveFilePath( CD[i][cnt] );
[16:04:42] <lostLinSoul> }
[16:05:13] <tomprince> I'd not write code like that .... too clever.
[16:05:35] <lostLinSoul> Why would one try resolving path a char a a time?...
[16:05:41] <fuzzie> gemrb definitely comes with a "this code is quite often terrible" warning
[16:05:47] <fuzzie> but CD is an array of an array of strings
[16:06:31] <lostLinSoul> I though it was an array of vectorys of type string
[16:06:40] <tomprince> And, with code written by different people that have widely divergent code styles.
[16:07:21] <tomprince> Yes.
[16:07:35] <tomprince> A vector is essentialy a fancy array.
[16:08:04] <lostLinSoul> Don't you have to use '.at' on vectors?
[16:08:23] <tomprince> nope.
[16:08:42] <tomprince> .at is bounds checked, [] isn't.
[16:09:29] <lostLinSoul> Regardless, I'm not mistaning in believing the the path is being revoved starting the the last char, then the last 2, then three, then 4, etc. chars
[16:10:11] <tomprince> The second [] is indexing into the vector, so the result of that is a string.
[16:10:15] <tomprince> Not a character.
[16:12:35] <lostLinSoul> Say my path was CD6/data, then it would start with "a", followed by "ta", then "ata", etc. no?
[16:13:18] <tomprince> no.
[16:13:58] <tomprince> CD[i] is an vector of strings, so cd[i][cnt] is a string, not a character of a string, and certainly not the tail of a string.
[16:14:31] <lostLinSoul> Sorry I meant if CD[0] contained "CD6/data" for example
[16:15:19] <tomprince> CD[0] *won't* contai "CD6/data", but it might containt { "CD6/data" }, which is a vector with one element.
[16:15:41] <lostLinSoul> Wouldn't CD[0][CD[i].size()] be '/0' ?
[16:16:18] <tomprince> No - $p:NC_2$, then $p:
[16:16:40] <tomprince> No. Because CD[0] *is not* a string.
[16:16:57] <lostLinSoul> ...
[16:17:48] <tomprince> CD is an array of vectors of strings.
[16:17:58] <tomprince> CD[i] is a vector of strings
[16:18:08] <tomprince> CD[i][j] is a string
[16:18:18] <tomprince> CD[i][j][k] would be a character.
[16:18:22] <lostLinSoul> Wait I may have made an incorrect assumption in that case
[16:18:54] <lostLinSoul> so for some reason, the program is storing more than one path for CD1?
[16:19:26] <lostLinSoul> Ist it possible to specify a path for CD1 more than once in the cfg?
[16:20:01] <tomprince> Yes.
[16:20:05] <lostLinSoul> I'm with you now
[16:20:41] <lostLinSoul> ....Why?
[16:20:46] <tomprince> Speaking of which ...
[16:22:22] <lostLinSoul> Of which ....?
[16:26:00] <CIA-28> GemRB: 03tom.prince * r7a66ef4b9249 10gemrb/gemrb/core/System/Logging.h: Factor out ifdef's for handling printf format attributes.
[16:26:00] <tomprince> v------- this.
[16:26:10] <CIA-28> GemRB: 03tom.prince * rcebc9de5bc42 10gemrb/gemrb/core/Interface.cpp:
[16:26:10] <CIA-28> GemRB: Support GoG cd paths out of the box.
[16:26:10] <CIA-28> GemRB: GoG puts the cd folders inside data. Add those as default CD paths.
[16:29:07] <lostLinSoul> What's GoG?
[16:29:27] <Kiranos> goodoldgames.com
[16:31:23] <lostLinSoul> ta I have trouble with all the TLA that hover around everything these days.....
[16:31:50] --> brad_a has joined #gemrb
[16:33:55] <lostLinSoul> StupidityDetector(), I like the naming sense! :0D
[16:48:44] <brad_a> heh i noticed that a few days ago myself
[16:50:16] <DrMcCoy> gemrb/core/Scriptable/PCStatStruct.cpp:167:34: error: ‘idx’ may be used uninitialized in this function [-Werror=uninitialized]
[16:50:53] <fuzzie> error() not annotated as noreturn?
[16:51:28] <brad_a> sounds like some new syntax
[16:51:42] <brad_a> or at least new to me :p
[16:54:22] <DrMcCoy> Nope, only as exported and printf syntax
[16:57:25] <-- brad_a has left IRC (Quit: brad_a)
[17:28:40] <-- SiENcE has left IRC (Quit: @all: cya)
[17:59:40] <-- Kiranos has left IRC (Quit: No Ping reply in 180 seconds.)
[18:00:01] --> Kiranos has joined #gemrb
[18:18:05] <lynxlynxlynx> tomprince: https://github.com/berenm/gemrb this could also be merged under the root
[18:18:28] <lynxlynxlynx> looks like only the debian folder is new + .gitignore changes
[18:19:58] --> Avenger has joined #gemrb
[18:20:16] --- ChanServ gives channel operator status to Avenger
[18:20:18] <Avenger> hi
[18:20:25] <tomprince> I suspect that we'll have to ask the owner to get it reparented.
[18:20:52] <tomprince> But, I'm still waiting on github to add support for reparenting things with seperate roots.
[18:21:03] <lynxlynxlynx> i'm not sure if we shouldn't just include it
[18:21:18] <lynxlynxlynx> lots of duplications with the configs
[18:22:00] <Avenger> no one fixed the compile error? : /home/laci/gemrb/gemrb/core/Scriptable/PCStatStruct.cpp:156: error: ‘idx’ may be used uninitialized in this function
[18:23:30] <lynxlynxlynx> apparently not
[18:23:57] <tomprince> I haven't had a chance to figure out the proper gcc incantation yet.
[18:24:44] <tomprince> Assuming fuzzie's diagnosis is correct.
[18:27:09] <Avenger> this is something different in msvc?
[18:27:11] <Avenger> meh
[18:27:18] <Avenger> msvc needs __declspec(noreturn)
[18:27:43] <tomprince> Okay. I can add that when I add gcc support.
[18:27:47] <Avenger> gcc needs maybe __attribute__ noreturn
[18:27:54] <tomprince> Probably.
[18:29:39] <Avenger> also: The attribute noreturn is not implemented in GNU C versions earlier than 2.5.
[18:30:02] <Avenger> so cool, we have to make loops and still stay incompatible with some compilers
[18:30:43] --> brad_a has joined #gemrb
[18:30:44] <tomprince> We don't support anything before 4.2, I don't think.
[18:31:07] <tomprince> And I think we discovered that 4.3 is broken wrt gemrb.
[18:32:12] <Avenger> ok, i think GEM_EXPORT void error(const char* owner, const char* message, ...) __attribute__((noreturn)) works
[18:32:56] <brad_a> 4.3 is broken, but we could esily work around it if we wanted to
[18:33:16] <Avenger> what uses 4.3?
[18:33:26] <tomprince> Yes. But why? ... exactly.
[18:33:48] <Avenger> my question was not like that, though :D
[18:34:08] <Avenger> how did we find out
[18:34:11] <fuzzie> is there a question?
[18:34:17] <fuzzie> oh, wjp was using 4.3.
[18:34:20] <fuzzie> i think.
[18:34:27] <tomprince> yep.
[18:34:35] <Avenger> so, we forced him to upgrade?
[18:34:43] <fuzzie> yes.
[18:34:54] <Avenger> i hope everyone can do that
[18:35:06] <tomprince> He could have downgraded to.
[18:35:11] <Avenger> i can't really upgrade msvc6, because it is the best :D
[18:35:43] <tomprince> :/
[18:35:43] <fuzzie> yes, indeed, msvc6 is top of the pile for obscure compiler bugs :)
[18:35:57] <Avenger> msvc7 and above are increasingly like tomprince :P
[18:36:12] <fuzzie> msvc10 compiler is really really nice
[18:37:10] <fuzzie> msvc2008 IDE was getting there, too.
[18:37:15] <fuzzie> alas.
[18:38:14] <Avenger> i agree that 6 got some annoying compiler bugs, also some debugger problems, like breakpoints cleared in dll's
[18:38:49] <fuzzie> "not awful" is also conspiciously missing from the feature lists for the vc11 ide
[18:39:12] <Avenger> so you recommend which version
[18:39:40] <fuzzie> i recommend crying :(
[18:40:48] <fuzzie> msvc2008 is worth a try for the IDE though, it was really getting there
[18:40:57] <Avenger> i would like the debugger and compiler of msvc7 with the gui + gui editor of msvc6.
[18:41:18] <Avenger> i don't think i ever tried to go above those
[18:41:28] <fuzzie> they rewrote the vc2010 IDE with WPF etc, so it's terrible and slow and buggy under WinXP, and has random waits of several minutes while it tries to get un-confused
[18:41:48] <fuzzie> but: they added a 'cancel' button to the dialogs for random waits of several minutes in the msvc2010 service pack.
[18:41:56] <Avenger> LOL
[18:42:00] <fuzzie> seriously.
[18:42:23] <Avenger> i would cancel the payroll of those developers instead
[18:42:26] <fuzzie> yes, well
[18:43:31] <CIA-28> GemRB: 03tom.prince * r64084abbfe65 10gemrb/gemrb/core/System/Logging.h: Annotate error as being noreturn.
[18:43:46] <fuzzie> but vc2008 is fast and debugger is fine
[18:44:11] <fuzzie> but no MFC in express so you'd have to keep msvc6 around
[18:44:17] <Avenger> hope declspec works at the end of line
[18:44:24] <Avenger> i usually see it in the beginning
[18:44:40] <fuzzie> well we'll find out, thanks to buildbot :)
[18:45:03] <Avenger> yeah, that's a cool thing
[18:47:12] <Avenger> no it isn't working: C:\buildslave\gemrb\msvc++6\build\gemrb\core\System/Logging.h(62) : error C2146: syntax error : missing ';' before identifier 'PRINTF_FORMAT'
[18:47:36] <tomprince> .... except that I have a broken version of buildbot installed right now, it seems.
[18:49:05] <CIA-28> GemRB: 03tom.prince * r9a605e0ecb6a 10gemrb/gemrb/core/System/Logging.h: Fix typo in PRINTF_FORMAT definition for non-gcc.
[18:52:57] <-- gembot has left IRC (Remote host closed the connection)
[18:53:15] --> gembot has joined #gemrb
[18:57:23] <-- Avenger has left IRC (Quit: bye!)
[19:08:02] <-- brad_a has left IRC (Quit: brad_a)
[19:09:20] --> Avenger has joined #gemrb
[19:09:20] --- ChanServ gives channel operator status to Avenger
[19:09:58] <Avenger> tom, i think the win32 logger is not working, because Logger* (*createDefaultLogger)() = createStdioLogger; is used?
[19:12:39] <CIA-28> GemRB: 03tom.prince * re34c581f2ae8 10gemrb/gemrb/core/System/Logger.cpp:
[19:12:39] <CIA-28> GemRB: Actually use console logger on win32.
[19:12:39] <CIA-28> GemRB: The code was accidentally using the stdio logger instead.
[19:13:03] <Avenger> i still don't see anything logged, though
[19:13:13] <Avenger> what is calling 'InitializeLogging' ?
[19:17:28] <Avenger> also, in error, move ShutdownLogging into the if to avoid an additional null pointer
[19:18:17] --> brad_a has joined #gemrb
[19:18:48] <tomprince> InitializeLogging is called by main
[19:19:12] <tomprince> Also, I've fixed ShutdownLogging in a branch.
[19:21:28] <Avenger> yeah, my linux -> win copy doesn't move gemrb.cpp
[19:22:28] <Avenger> ok, logging works now :)
[19:24:34] <Avenger> do you plan on letting people change the logger? or it is already possible somehow?
[19:25:21] <tomprince> Yes and no.
[19:27:50] <CIA-28> GemRB: 03tom.prince * rc8840162ab9d 10gemrb/gemrb/core/System/ (Logging.cpp Logging.h): Add support for multiple loggers.
[19:27:52] <CIA-28> GemRB: 03tom.prince * r36573bda4c95 10gemrb/gemrb/core/System/Logging.h:
[19:27:52] <CIA-28> GemRB: Move NORETURN to before definition.
[19:27:52] <CIA-28> GemRB: MSVC expects __declspeci((return)) to be before the definition.
[19:41:41] <gembot> build #479 of msvc++6 is complete: Success [3build successful] Build details are at http://buildbot.gemrb.org/builders/msvc%2B%2B6/builds/479
[19:43:20] <tomprince> ^--- and gembot is working again.
[19:45:30] <lynxlynxlynx> oh right, did anyone check if there's any soft limit on uploads?
[19:45:43] <lynxlynxlynx> the builds are small, so i'm not that worried (cf. wesnoth)
[19:45:55] <lynxlynxlynx> but still it would be nice to be nice
[19:49:45] <tomprince> I just asked now in #sourceforge
[20:00:59] <gembot> build #253 of nmake-msvc++6 is complete: Success [3build successful] Build details are at http://buildbot.gemrb.org/builders/nmake-msvc%2B%2B6/builds/253
[20:01:53] --> SiENcE has joined #gemrb
[20:03:36] <tomprince> <@ctsai-sf> Okay, those are fairly small. In terms of disk space, if you're taking up several gigs, it's probably time to prune back, but it seems that you're a ways away from that. With that said though, it seems to me that with releases that often, each individual release doesn't seem to have much value, so you may want to consider removing old files for that reason
[20:12:34] <tomprince> <@ctsai-sf> If you keep up this pace, I would recommend a pruning of old "nightly" builds every month or two. Leaving all the major releases should be fine.
[20:16:07] <Avenger> meh, finally found the itemtype->weapon prof mapping in HoW
[20:19:12] <lynxlynxlynx> thanks
[20:19:28] <lynxlynxlynx> iirc it can be done via ssh now, so it should be trivial
[20:33:57] <gembot> build #197 of nmake-msvc++10 is complete: Success [3build successful] Build details are at http://buildbot.gemrb.org/builders/nmake-msvc%2B%2B10/builds/197
[20:39:22] <brad_a> can i inturrupt everybody for a min so we can discuss this: https://github.com/bradallred/gemrb/tree/SDL
[20:42:10] <fuzzie> difficult to review
[20:46:40] <brad_a> im sorry :(
[20:58:06] <-- Avenger has left IRC (Quit: bye!)
[21:03:42] <CIA-28> GemRB: 03avenger_teambg * r0d108bcdf0b0 10gemrb/gemrb/override/ (how/proftype.2da iwd/proftype.2da): weapon proficiencies mapping for iwd
[21:11:26] <lynxlynxlynx> :)
[21:26:17] <-- Yoshimo has left IRC (Quit: Yoshimo)
[21:27:20] <brad_a> i like all the '\r' characters :-p
[21:41:26] <lynxlynxlynx> brad_a: it's not clear from your post if the list applies to the old or the new builds
[21:42:08] <brad_a> it will apply only after merging my SDL branch. but currently builds are broken because i have upgraded my sdl library for ios
[21:42:25] <brad_a> so they wont work again until the sdl 2 driver is available
[21:46:32] <brad_a> i was hoping to merge that today, but i though i would let people look at it first
[21:47:07] <lynxlynxlynx> it's out of my scope and the diff is not small
[21:48:52] <brad_a> yeah but its mostly cut and paste
[21:50:29] <lynxlynxlynx> was just about to say it is probably much smaller if you account for moves
[21:51:37] <tomprince> It might be interesting to look at in git gui blame
[21:52:37] <brad_a> a lot of code will die when i re engineer the event queue. but i think that needs to be a separate commit
[21:57:45] <lynxlynxlynx> or if it was copied first and then the bulk disappears with -B
[22:25:27] <tomprince> I'd rather have less code accesing core, than more, so I don't like removing the SetDisplayTitle.
[22:25:46] <tomprince> Although, I guess passing it to createdisplay or something would be fine, too.
[22:26:56] <brad_a> then thats what i will do
[22:27:10] <brad_a> i dont mind having settitle other than that it is used just once
[22:30:14] <tomprince> Also, you still have version defines, after the split.
[22:32:04] <brad_a> yeah but there isnt a good way right now to make SDLVideo work with both 1.2 and 2.0 without those few macros
[22:32:51] <brad_a> i should, however, remove the poolEvents from base until i get around to implementing new event handling
[22:34:38] <CIA-28> GemRB: 03tom.prince * rb5d132e584e9 10gemrb/gemrb/core/ (CMakeLists.txt System/Logger/File.cpp System/Logger/File.h): Add file logging.
[22:34:45] <CIA-28> GemRB: 03tom.prince * r6983e90977f7 10gemrb/gemrb/core/System/ (Logger.cpp Logger/Apple.h Logger/Apple.mm):
[22:34:45] <CIA-28> GemRB: Remove Apple logger.
[22:34:45] <CIA-28> GemRB: All this does is override display color, which can better be handled
[22:34:45] <CIA-28> GemRB: by passing NOCOLOR to cmake.
[22:43:49] <-- lynxlynxlynx has left IRC (Remote host closed the connection)
[22:45:05] <tomprince> Also, I don't like the static local vars, although I realize that is your fault.
[22:48:27] <brad_a> i didnt make any new ones… and i am rewriting the touch input code right after this gets commited
[22:51:09] <tomprince> We now have three copies of PollEvents in your code?
[22:51:37] <brad_a> yes as i said it needs to be rewritten. i was going to do that as a subsequent commit
[22:52:04] <tomprince> Well, we should at most have 2 then.
[22:52:49] <brad_a> i know xactly what needs to be done. right now im rebaseing and removing the code from the base class
[22:53:12] <brad_a> but new work needs to be done to distill it down the way it needs to be
[22:53:38] <tomprince> But there should be 1 or 2 copies of any given bit of code, not 3.
[22:54:25] <brad_a> i just said i removed the base code for now
[22:54:52] <tomprince> Ahh.
[22:54:59] <brad_a> i am aware this isnt how it needs to be in the end but there is no reason to do all this event rewriting in the same commit
[22:56:06] <tomprince> No, certainly not in one commit.
[22:59:24] <tomprince> I wonder if the proper way to handle this would be to have the polling in the base class, but pass the message to the subclass and give it a chance to handle the message, before handlng the common events in the base.
[23:00:56] <brad_a> thats exactly what i was going to do :)
[23:02:55] <tomprince> Sounds good. It'd probably be clearer if you factor that code in two directly, rather than copying it twice and then refactoring it back together.
[23:03:23] <fuzzie> but annoying :p
[23:08:02] <brad_a> well i wanted to do that refactoring at the same time as fixing touch input
[23:08:18] <brad_a> so for now id rather just have 2 versions of pollevents in both drivers
[23:18:14] <brad_a> https://github.com/bradallred/gemrb/tree/SDL
[23:18:28] <brad_a> ok removed pollevents from base for now
[23:18:55] <brad_a> and made createDisplay take a title param
[23:19:35] <brad_a> hmm i can remove one more of those defines now that pollEvents is gone
[23:21:04] <brad_a> k done
[23:26:13] <tomprince> fuzzie: Why annoying?
[23:27:12] <tomprince> I'd think less annoying: a pure refactoring. (Not in the same commit as the rest of the split, obviously.
[23:29:06] <-- SiENcE has left IRC (Quit: cya)