#gemrb@irc.freenode.net logs for 10 May 2011 (GMT)

Archive Today Yesterday Tomorrow
GemRB homepage


[00:32:00] <-- Maighstir has left IRC (Quit: ~ Trillian Astra - www.trillian.im ~)
[01:21:47] --> _pickle has joined #gemrb
[01:54:16] <tomprince> dhewg: HashMap::take looks broken, as it returns a pointer to a (logically) freed value.
[01:58:51] <tomprince> I think the StringMap is reasonable.
[02:00:31] <tomprince> It smells wrong, having to subclass. But I don't have any better ideas, so I think this can go in as is. And if/when we figure out something better, the interface isn't going to be different, anyway.
[02:02:11] <-- _pickle has left #gemrb ("Konversation terminated!")
[02:04:48] <tomprince> I can't and won't comment on the internal details. (Except to wonder about deque/vector)
[02:06:18] <tomprince> init/setDescription should probably be part of the constructor.
[02:07:35] <tomprince> at the very least, if init needs to be called before the hash is useful, the constructor needs to call it, with sane defaults.
[02:11:48] <tomprince> I'd say don't bother checking for NULL from new, since we don't turn exceptions off, it will never return NULL. And if we were, we would need to audit the entire code base anyway.
[02:14:27] <tomprince> I'd probably inline incAccess by hand. That makes it clear that it is debuging code.
[02:17:21] <tomprince> drop the virtual on the destructor, 1) the only subclass has just calls the base class one, and nothing else 2) It is never going to be used polymorphicly, so the compiler is going to know the type anyway.
[02:19:31] <tomprince> Drop the inline in the class definition. If it going to be anyware, it should be on the function definitions themselves, but the compiler basicly ignores it anyway.
[02:21:38] <tomprince> _hash should be a typedef, and the methods on HashKey static.
[02:23:56] <tomprince> I'd say more stuff should be private, not protected.
[02:30:35] <tomprince> Do we need a description? It seems irrelevant.
[02:31:17] <tomprince> re: init, I think it would make sense for HashMap to always be in an initialized state, so the IsIntialized checks could go.
[02:32:48] <tomprince> You should specialized just the functions of HashKey, not the entire thing, except for HashKey<std::string>
[02:33:11] <tomprince> (and you semm to be missing hash(char*) in that case)
[02:40:00] <tomprince> Other than that, it looks good to merge.
[02:41:17] <tomprince> I haven't looked at the commits that use it in detail, but at a glance, they look reasonable.
[02:45:56] <tomprince> (perhaps take can just go away, since nothing in tree uses it right now)
[02:47:45] <tomprince> And you should have a comment on why the char overrides to HashKey are commented out. Something to the effect that msvc thinks char == char[0] for template specialization purposes.
[02:48:31] <tomprince> (lots of comments, but mostly trivial)
[06:03:31] --> lynxlynxlynx has joined #gemrb
[06:03:32] --- ChanServ gives channel operator status to lynxlynxlynx
[06:37:39] <dhewg> tomprince: alright, thanks for the comments
[06:37:55] <dhewg> there're a few reasons behind this little things you mentioned
[06:38:23] <dhewg> the decription is mostly for the debug stats, if you try it with that define you'll see
[06:38:53] <dhewg> i seperated init() from the constructor later on, because the uses are more easy that way
[06:39:12] <dhewg> e.g. the bif/key importers init it with a size they do not know beforehand
[06:39:24] <dhewg> so it avoid foo = new HashMap()
[06:40:33] <dhewg> and yeah, i was aware that take() returns a pointer that will be reused after following inserts. it can prolly go, i dunno if something might use it in the future
[06:42:23] <dhewg> and the incAccesss thing is just for derived classes, so the stats are correct :)
[06:44:24] --> Bo_Thomsen has joined #gemrb
[07:05:16] --> lubos has joined #gemrb
[07:13:57] <-- mihairu has left IRC (Ping timeout: 258 seconds)
[07:15:19] <-- Bo_Thomsen has left IRC (Quit: Leaving.)
[07:16:25] --> adominguez has joined #gemrb
[07:28:32] <dhewg> and the incAccesss thing is just for derived classes, so the stats are correct :)
[07:28:35] <dhewg> meep
[07:31:22] <fuzzie> oh ah.
[07:32:17] <dhewg> ^A^A+cursor up+enter on the wrong window :)
[07:39:41] <fuzzie> :)
[07:49:03] <dhewg> fuzzie: and yes, toee is impressive. the 3d creature stuff on the 2d bg fits nicely
[07:49:40] <fuzzie> well, the inside stuff i've seen still looks bad
[07:50:46] <-- devurandom has left IRC (Remote host closed the connection)
[07:50:57] <fuzzie> but apparently it *can* look good. but my point with the screenshot was to be impressed at the reimplementation. :P
[07:53:57] --> devurandom has joined #gemrb
[08:01:48] <dhewg> ah, that the failsharp thing?
[08:02:10] <fuzzie> it seems to have been ported to C++/Qt/etc
[08:02:16] <dhewg> uhhh
[08:02:18] <dhewg> nice
[08:02:35] <dhewg> i just stumbled upon it because its from some dudes of the community fix pack
[08:04:37] <fuzzie> welll, it also seems mostly dead, ofc
[08:04:48] --> SiENcE has joined #gemrb
[08:06:56] <fuzzie> it's a pity that it would be so absurdly much work to add support for a non-IE game
[08:17:30] <dhewg> well thats the price for a project that is designed for only one engine
[08:17:54] <dhewg> lets do a scummvm style thing for isometric rpgs?
[08:18:58] <fuzzie> not very viable i think :)
[08:21:00] <dhewg> ahh come on, one central scripting VM with a recompiler for all the other crap out there is totally trivial!
[08:21:36] <fuzzie> :-p
[08:21:55] <fuzzie> you find a way to make something like that even remotely start to work, a lot of reimplementation project would be blissfully happy
[08:24:50] <dhewg> heh
[08:25:43] <dhewg> well, its likely to be painful, but a static recompiler that produces stuff for runtime would be less painfull than to do that dynamically at runtime
[08:26:01] <dhewg> but i think most reimplementation aims at the latter
[08:26:17] <fuzzie> yeah, but i think in the end, you'd be better off with static subclasses in pretty much all cases i've seen
[08:26:57] <fuzzie> like, most of the IE script bugs we have left are because of the 10 different codepaths via which the script actions are executed
[08:27:49] <fuzzie> all of which are dictated by the game logic, rather than anything at script level
[08:30:13] <dhewg> huh, fun
[08:30:40] <fuzzie> i am hoping we can mostly ignore them
[08:30:46] <fuzzie> via hax
[08:43:54] <edheldil> dhewg: also please fix the year in the boilerplate
[08:48:24] <wjp> hi
[08:48:39] <fuzzie> wjp: made it home? :)
[08:48:50] <wjp> yeah, close to midnight
[08:54:54] <edheldil> I wonder if it would be possible to have a pre-* hook in git which would check c++ and python files for correct boilerplate.
[08:57:25] <fuzzie> do we have a way of checking for correct boilerplate at all?
[08:59:06] <-- |Cable| has left IRC (Remote host closed the connection)
[08:59:44] <edheldil> for some values of "correct", there's a script in admin dir. But I am not so sure of how it would work with git
[08:59:50] <fuzzie> well
[09:00:00] <fuzzie> it seems to just check that everything matches exactly with some GemRB copyright
[09:00:15] <edheldil> sure, it's outdated and stupid
[09:00:34] <fuzzie> which is not very useful, since it gives a lot of complaints about e.g. ffmpeg-copyrighted code
[09:01:14] <edheldil> it could be amended for that part of our tree
[09:05:58] <edheldil> but integrating it with buildbot would be easier
[09:06:32] <fuzzie> what would buildbot do, though?
[09:07:35] <fuzzie> i mean, if we have no correct checking in the first place
[09:15:09] --> mihairu has joined #gemrb
[10:07:25] <dhewg> edheldil: k, done
[11:22:40] <tomprince> If the debug stuff sticks around, I'd remove setDescription and make dumpStats public.
[11:25:25] <-- adominguez has left IRC (Remote host closed the connection)
[11:35:35] <tomprince> And document the magic numbers you use for limits.
[12:05:08] --> adominguez has joined #gemrb
[13:01:33] <fuzzie> that takes away the magic!
[13:05:42] <tomprince> I like my magic in the game. not the code. ;)
[13:06:36] <dhewg> which magic numbers now?
[13:06:46] <tomprince> in the init calls.
[13:07:32] <dhewg> well those are just numbers i pulled out of my ass :P
[13:08:07] <tomprince> Then why are they all different?
[13:08:07] <dhewg> i mean, i tried a couple, and those gave a "good looking" distribution and not too many empty buckets
[13:08:22] <dhewg> they're probably not the best choice
[13:09:03] <dhewg> with the 32k in the keyimporter the map uses 1.2mb or so
[13:09:14] <dhewg> so that limits the memsize a little too
[13:09:39] <dhewg> (that's for bg2 with >40k entries)
[13:10:58] <tomprince> Well, then perhaps you should perhaps add a comment that that was how they were picked, and give approximate numbers.
[13:12:16] <dhewg> sounds like a good idea
[13:12:37] <dhewg> but even better would be if maybe someone else could try a few things too?
[13:14:23] <fuzzie> should probably test them with non-bg2
[13:17:38] <dhewg> that would be nice
[13:39:26] <-- mihairu has left IRC (Quit: mihairu)
[13:41:04] --> mihairu has joined #gemrb
[13:49:32] <-- mihairu has left IRC (Remote host closed the connection)
[13:50:08] --> mihairu has joined #gemrb
[14:01:48] <edheldil> dhewg: is it for full filenames or just resrefs?
[14:05:27] <-- mihairu has left #gemrb
[14:33:23] <dhewg> the hashmap itself is a template, you can stuff anything you want in it. for the keyimporter its doing the same as the current code, which is a combination of resref+type
[15:30:26] <-- lubos has left IRC (Quit: Leaving.)
[15:41:59] --> test32894789234u has joined #gemrb
[16:00:40] --> mihairu has joined #gemrb
[16:05:00] <-- test32894789234u has left #gemrb
[16:05:32] --> test32894789234u has joined #gemrb
[16:16:23] <-- SiENcE has left IRC (Quit: @all: cya)
[17:09:08] --> Bo_Thomsen has joined #gemrb
[17:11:10] <-- mihairu has left IRC (Quit: mihairu)
[17:12:06] --> mihairu has joined #gemrb
[17:30:45] --> |Cable| has joined #gemrb
[17:38:26] <-- adominguez has left IRC (Quit: Saliendo)
[17:55:27] <gembot> build #141 of msvc++6 is complete: Failure [failed compile] Build details are at http://buildbot.gemrb.org/builders/msvc%2B%2B6/builds/141 blamelist: dhewg <dhewg@wiibrew.org>
[17:55:27] <gembot> build #141 of msvc++6 is complete: Failure [failed compile] Build details are at http://buildbot.gemrb.org/builders/msvc%2B%2B6/builds/141 blamelist: dhewg <dhewg@wiibrew.org>
[17:55:38] <dhewg> my bad :P
[17:59:18] <dhewg> but i dont get it
[18:00:51] <fuzzie> as a mental note, the original engine truncates tooltips if needed
[18:01:24] <dhewg> hm?
[18:01:37] <fuzzie> if the text is too long, it just cuts it off
[18:02:18] <dhewg> is that related to any of my commits?
[18:02:48] <fuzzie> no
[18:02:51] <fuzzie> just mental note :)
[18:03:02] <fuzzie> what's it complaining about, the template<unsigned int size> struct<char[size]>?
[18:03:04] <dhewg> k, was confused there for a moment :)
[18:03:10] <dhewg> yeah
[18:03:23] <dhewg> im not sure what it wants with [0] though
[18:03:42] <dhewg> i can disable that block now, because its unused atm
[18:03:51] <dhewg> but the plan was to use that for the variables
[18:03:58] <dhewg> it uses a max len of 40
[18:04:35] <fuzzie> i mean, if i'm not mistaken, that's a partial specialized template
[18:04:50] <dhewg> yeah
[18:04:55] <fuzzie> and thus, no support
[18:05:04] <dhewg> like... at all?
[18:05:09] <fuzzie> nope
[18:05:13] <dhewg> great
[18:05:26] <fuzzie> template specialisations must be fully specified
[18:05:37] <fuzzie> i did warn about it :)
[18:06:16] <dhewg> yeah well, i dunno if we need more than one 'size'
[18:06:34] <dhewg> so for now a fixed 40 thingy can be used in variables.cpp
[18:06:47] <dhewg> but still... :P
[18:08:05] <dhewg> if you're bored look at my variables patch :)
[18:08:26] <dhewg> just a little refactoring, but im scared of conflicts :P
[18:09:26] <fuzzie> i forget when vc++ became saner
[18:09:45] <dhewg> wise choise of words :)
[18:10:05] <fuzzie> i guess maybe vc++ 7.1
[18:10:50] <fuzzie> i'm not sure about the variables patch
[18:10:53] <tomprince> :( to more stuff hanging off Interface. But a return value rather than a pointer paramater.
[18:11:02] <tomprince> ... is good.
[18:11:41] <fuzzie> well
[18:11:42] <fuzzie> is it?
[18:12:13] <fuzzie> it's difficult to follow what's going on at a glance
[18:12:27] <dhewg> its a preparation for variables, but even if you skip that thought, whats not good about exposing less internals?
[18:13:02] <tomprince> I have been waging a war against having functionality in Interface.
[18:13:18] <fuzzie> but what should be in Interface?
[18:13:39] <tomprince> Not much of anything.
[18:13:52] <tomprince> (not everybody agress :))
[18:13:54] <fuzzie> i think i object to the idea of removing the interface from Interface
[18:14:15] <fuzzie> but it's not at all clear what you want, so :)
[18:15:46] <fuzzie> in any case, s/setVariable/SetVariable/ given the style of everything else, and i'd rather the parameter be always explicit
[18:15:56] <tomprince> For a start, avoid adding more stuff to it.
[18:16:07] <fuzzie> and put it where?
[18:16:20] <tomprince> Free functions?
[18:16:29] <fuzzie> ok, then definitely not agreeing :P
[18:16:41] <tomprince> Well, not for this in particular.
[18:16:48] <fuzzie> last thing i want is more globals
[18:16:58] <dhewg> singletons!
[18:17:04] <dhewg> wooooo, fancy!
[18:17:12] <tomprince> My argument is that everything in interface is already a global.
[18:17:22] <fuzzie> i think we have one singleton, and it is a global variable in disguise
[18:17:35] <fuzzie> and should die for that reason
[18:18:33] <tomprince> PluginMgr? That is the way it is because of static linking.
[18:18:39] <fuzzie> no
[18:18:51] <fuzzie> it's the way it is because it's a broken singleton :P
[18:19:12] <dhewg> heh
[18:19:32] <tomprince> Well, I have no attachment to its current implementation.
[18:19:33] <fuzzie> it should do a new on first use
[18:19:38] <fuzzie> just haven't gotten around to bashing at it yet
[18:20:18] <fuzzie> but it leads nicely to my objection here, which is that i disagree with adding more global variables with unreliable destructors, when you can have them in Interface and keep it simple
[18:21:18] <fuzzie> just seems like so far we've just gained a bunch of confusing hacks to zap global stuff in the right order, which just makes the code crazier
[18:21:26] <fuzzie> but, grumpy :P
[18:22:41] <fuzzie> this is not to say i wouldn't like all the random junk in Interface to live in other classes
[18:24:14] <gembot> build #142 of msvc++6 is complete: Success [build successful] Build details are at http://buildbot.gemrb.org/builders/msvc%2B%2B6/builds/142
[18:24:14] <gembot> build #142 of msvc++6 is complete: Success [build successful] Build details are at http://buildbot.gemrb.org/builders/msvc%2B%2B6/builds/142
[18:24:16] <tomprince> Ahh... so we agree then. Who'd thunk it. ;)
[18:26:10] <tomprince> Although I am not sure about what you want re: the paramater being explicit.
[18:26:36] <fuzzie> this is just in the case of this code
[18:26:59] <fuzzie> GetVariable("Name", 0) rather than GetVariable("Name"), if it's going to return the default like that
[18:28:16] <tomprince> Yes, that makes sense.
[18:29:15] <tomprince> My comment was against 'int someVariable = default; ...->GetAt("someVariable", &someVaraible);"
[18:29:20] <fuzzie> also some nitpicks, e.g. it would be nice if NeedsLevelUp stayed as a var name
[18:30:03] <dhewg> is it just this case or should i avoid default arguments altogether?
[18:30:30] <fuzzie> dhewg: i don't think there is a reasonable default here, so i'd prefer they be avoided
[18:30:44] <fuzzie> i worry that there will be places where we really want to know if a variable exists or not, too
[18:30:51] <fuzzie> i'm also surprised that only ieDwords are used..
[18:30:53] <dhewg> well its a ieDword map
[18:31:17] <dhewg> yeah, while the variable thing was 3 modes, its only one for each instance
[18:31:23] <dhewg> *has
[18:31:34] <fuzzie> that's quite great, but surprising
[18:32:06] <fuzzie> you've also removed the casts from the (ieDword)-1 stuff in some places
[18:32:07] <dhewg> well, i dont think 0 is an unreasonable default for unsigned stuff, but lemme change that
[18:33:16] <fuzzie> i think "don't fiddle with the code so much while replacing it" maybes covers all my concerns with it
[18:34:18] <-- |Cable| has left IRC (Remote host closed the connection)
[18:34:39] <tomprince> How about add the defaulting version of get to HashMap, and the just swap that in, leaving GetDictionary.
[18:34:56] <tomprince> Because I don't think adding wrapper functions to Interface makes things clearer.
[18:35:06] <dhewg> that would drag the template into almost everything
[18:35:45] --> |Cable| has joined #gemrb
[18:36:28] <fuzzie> i'm not sure what effect that has
[18:36:48] <fuzzie> i wish i had more time to look at it atm
[18:38:00] <dhewg> if you look at that patch, all that was ever done was to get and set variables. whats wrong with hiding that the dictionary or a hashmap is behind that?
[18:38:33] <tomprince> See my objection above to adding all sorts of random junk to Interface.
[18:38:34] <dhewg> its what i consider good coding practice, especially for big projects
[18:38:59] <tomprince> I don't mind having an abstraction for it, I do mind that abstraction being Interface.
[18:39:11] <fuzzie> with the disclaimer that i doubt it's relevant here: you're falling into the "only looking at the current use cases of a huge broken project" trap there, which is a bad idea
[18:39:27] <fuzzie> and i think there's not a better place for true engine globals than Interface
[18:42:38] <dhewg> uhm
[18:43:10] <dhewg> i dont think its broken, but i do see lots of places which can be improved
[18:44:05] <fuzzie> well, it's broken in the sense that it's still missing large amounts of important functionality, quite apart from design/refactoring issues
[18:44:48] <fuzzie> a more important point is maybe that we shouldn't have the dictionary at all
[18:44:58] <fuzzie> see, this is why i don't like thinking about things
[18:45:47] <dhewg> heh
[18:45:49] <tomprince> Creates more and disruptive work? :)
[18:45:59] <dhewg> that line is topic worthy
[18:46:22] <tomprince> I never did like that dictionary.
[18:46:39] <fuzzie> well, it's used for a bunch of things
[18:46:44] <fuzzie> it's used for the UI hacks, which should be de-hacked
[18:46:57] <fuzzie> and it's used for the options, which maybe are a legitimate use?
[18:47:23] <fuzzie> and it's used for the automatic control variable stuff, which overlaps with the options
[18:48:28] <tomprince> (options) Perhaps, although it means a hash lookup it a bunch of places that otherwise wouldn't need it.
[18:49:12] <tomprince> Automatic control variable stuff?
[18:49:39] <fuzzie> gui controls which set an associated variable of, say, "GUI Feedback Level"
[18:49:53] <fuzzie> and then the variable is modified as you move the slider
[18:50:01] <dhewg> i stumbled upon drag coords which are pushed in there
[18:50:24] <tomprince> Ah, yes. I would be inclined to say that that stuff should be more explicit, or delcarative or something.
[18:50:36] <tomprince> Same perhaps with options.
[18:50:51] <tomprince> Which is, of course, more and distruptive work. :P
[18:51:18] <dhewg> and in my excuse, i tried to just refactor without changing behaviour
[18:52:22] <tomprince> np.
[18:53:09] <tomprince> Part of it is just me an fuzzie pulling at right angles to one another. With gemrb be pulled vaguely down the middle.
[18:54:12] <fuzzie> it is the reason i tend to err towards not fiddling with things if at all possible
[18:54:37] <fuzzie> because my experience has shown that often, the moment we refactor one thing, it turns out that it's inappropriate for the other thing we need
[18:55:45] <tomprince> dhewg: I'm fairly certain your stuff will get merged, after another round or two.
[18:56:20] <fuzzie> well, i think we need the HashMap
[18:56:22] <fuzzie> lest we all go insane
[18:56:41] <tomprince> Speak for yourself. ;)
[18:56:46] <dhewg> even if its not, i wont hold a grudge, but im having a hard time to judge whats right or accepted
[18:57:49] <fuzzie> we can always reverse/change this stuff as needed
[18:57:52] <fuzzie> tomprince is really good with sed
[18:58:13] <dhewg> heh
[18:58:43] <tomprince> I have done some quite crazy things with sed on the codebase.
[18:58:58] <dhewg> anyway, maybe my idea of helping out to improve stuff is just different
[18:59:23] <fuzzie> so if you're annoyed by stuff like the efault params, maybe ask tomprince to fix it up
[18:59:30] <fuzzie> since it seems to be an awful lot less work for him
[18:59:50] <dhewg> no, thats just fine
[19:00:13] <fuzzie> my issue is really simply that i don't have time to actually mess with the code atm, as you can see by the lack of commits
[19:00:15] <dhewg> i dont care about that, its just a matter of taste
[19:01:14] <-- test32894789234u has left #gemrb
[19:03:33] <fuzzie> so all i can provide on irc is inane commentary
[19:10:04] <tomprince> dhewg: No one is in charge here, so you will never get a final offical yeah or nay, until someone commits it.
[19:10:45] * tomprince goes to make granola.
[19:11:14] <fuzzie> which is why i was mumbling in the first place about asking for commit access if you feel it's an issue
[19:11:18] <fuzzie> our speed, i mean
[19:11:36] <dhewg> lets better not do that :)
[19:11:46] <dhewg> see what i did to the android port
[19:11:49] <dhewg> of scummv
[19:11:52] <dhewg> +m
[19:12:20] <fuzzie> well, if you make gemrb work that well.. ;p
[19:12:25] <dhewg> its like a jammed it a few times through a meat grinder :P
[19:12:31] <dhewg> but i think for the better in the end
[19:16:27] <tomprince> fuzzie: what are your thoughts re: the debug code
[19:17:14] <tomprince> It seems to me that it is likely to bitrot.
[19:17:43] <fuzzie> i'm not sure it bitrotting is a bad thing
[19:18:23] <tomprince> I *don't* like setDescription. It feels like adding that to std::map, which would never fly.
[19:18:25] <fuzzie> as a 'good idea to have in a tree while we're still making sure it's good' thing
[19:20:07] <dhewg> the hashmap setDescription?
[19:20:33] <fuzzie> i don't really care about ugly debug stuff, honestly :P
[19:26:22] <tomprince> dhewg: yes. One thought was just make dumpStats public, and not call it anywhere. Then, when one is debugging, one can add calls where appropriate and pass in a description.
[19:28:13] <dhewg> with the bif and dir importers there're already a quite some hashmaps used. the idea was to make it easy to fiddle with e.g. the hash function and see what changes
[19:28:45] <dhewg> we can just drop setDescription i guess
[19:28:57] <dhewg> or only set it if the debug define is set
[19:29:15] <dhewg> in the end i dont really care :)
[19:29:31] <dhewg> right now its just easy to see how a change affects all the maps
[19:30:38] <dhewg> and for the record: i stole the idea to dump thats upon destruction from "wtf", webkit's web template framework :D
[19:32:29] <-- mihairu has left IRC (Ping timeout: 260 seconds)
[19:32:32] <tomprince> If you change that, and the other points I made and you didn't comment on, I'll merge it.
[19:33:17] <dhewg> i think i already pushed the minor things you mentioned
[19:33:27] <dhewg> as fixups
[19:33:43] <dhewg> how should i change it then?
[19:33:51] <dhewg> make the dump func public?
[19:45:29] <tomprince> That would be my suggestion.
[19:46:17] <fuzzie> i reserve the right to rewrite it ;p
[19:46:31] <fuzzie> but assuming it works fine, i see no point in keeping it in a branch
[19:47:03] <fuzzie> and if it doesn't work fine, we can always fix, or revert and fix
[19:47:06] <dhewg> but really, run it with that define set, and you'll see why its like this atm
[19:47:16] <tomprince> You are still specializing HashKey, rather than just the member functions.
[19:47:26] <fuzzie> but we can fix that later :P
[19:47:26] <tomprince> And _hash should be a typedef.
[19:47:44] <dhewg> ah right, i didnt understand the typedef suggestion
[19:48:00] <fuzzie> but it's ok if i revert this if it's terrible?
[19:48:23] <dhewg> well if you have doubts we dont need to rush it
[19:48:31] <fuzzie> mostly i am concerned about impact of the BIFImporter performance-wise
[19:48:52] <dhewg> that would be my last concern with that :)
[19:48:53] <fuzzie> which is in a single nice commit afaict
[19:49:35] <fuzzie> well, i don't know, it looks super slow atm :P
[19:49:54] <tomprince> fuzzie may disagree with me on this, but I'd say drop the checks on new.
[19:50:45] <dhewg> fuzzie: "looks slow"?
[19:50:45] <fuzzie> in allocBlock?
[19:50:56] <fuzzie> dhewg: you're creating a whole hash table for every file access, no?
[19:51:04] <fuzzie> looks *super* slow. :P
[19:51:22] <dhewg> well then dont pick the bif commit
[19:51:22] <tomprince> there was a couple of == NULL checks on the return value of new.
[19:51:57] <dhewg> thats why i keep talking about a mru like bif cache
[19:52:17] <fuzzie> yes, it makes sense in long-term
[19:52:24] <fuzzie> i just don't know what effect it has in short-term
[19:52:32] <fuzzie> i am not very happy with keeping code out-of-tree though, it gets lost too much
[19:52:38] <fuzzie> and is a pain for everyone with rebasing etc
[19:52:45] <tomprince> And, like I said, if inline belongs anywhere it is on the definition, not the declaration.
[19:53:16] <tomprince> (I think, though if anybody disagrees, i won't stick on that point)
[19:53:37] <fuzzie> well, i think, it's such a complex patch set that you can keep coming up with issues forever
[19:53:54] <fuzzie> and you can always commit, and then fiddle
[19:54:26] <dhewg> as for the inline, im used to do it like on that patch
[19:54:47] <dhewg> do all compiler like it when you strip it on the declaration?
[19:55:57] <tomprince> I haven't tested it, but that is my smallest nit. It's not like the compiler pays much attention to it anyway, as far as I can tell.
[19:56:10] <dhewg> heh :)
[19:56:51] <dhewg> as i see it, give it less excuses to moan about different declarations/definitions :D
[19:56:53] <fuzzie> 'sort_of_maybe_inline_this_if_you_think_its_a_good_idea' being a bit too long
[19:58:48] --> SiENcE has joined #gemrb
[20:09:01] --> SiENcE_ has joined #gemrb
[20:12:33] <-- SiENcE has left IRC (Ping timeout: 240 seconds)
[20:41:43] --> Beh0lder has joined #gemrb
[20:42:24] <Beh0lder> hi all
[20:45:26] <Beh0lder> please check my Buglist, maybe anything is been fixed?
[20:45:39] <-- Beh0lder has left #gemrb
[20:45:47] --> Beh0lder has joined #gemrb
[20:53:20] --> barra_home has joined #gemrb
[21:01:27] <lynxlynxlynx> i doubt it, you have mostly bg1 stuff on there
[21:03:38] <-- Beh0lder has left IRC (Read error: Connection reset by peer)
[22:00:51] <-- lynxlynxlynx has left IRC (Remote host closed the connection)
[22:23:36] <-- barra_home has left IRC (Quit: Verlassend)
[22:38:09] <-- Bo_Thomsen has left IRC (Quit: Leaving.)
[22:41:37] <-- SiENcE_ has left IRC (Quit: cya)