Except that this commit does everything I need it to, short of cross-frame compression. In fact, it even speeds EFF loading up.. by a lot. As in, my Bastion mainhall (which used to take 3+ seconds to load) now loads up in a fraction of the time.
I don't doubt that Valathil's changes are a significant improvement on the features in question. What I'm trying to find out is its potential effects on the other parts of the code, and to what extent people were looking out for that sort of thing. The law of unintended consequences is merciless.
Secondly, I know you don't like this, but Valathil was very open about discussing what he was doing in this patch on IRC. I dare say that the people most qualified to evaluate this fix were listening, and agreeing with the steps taken here.
What I don't like is the lack of a permanent record of the discussion or a notification that a discussion even took place. But the fact that a discussion did take place, with multiple participants, and with multiple qualified eyes on the problem, is what I was primarily concerned about.
Consider that anybody who wasn't present at the IRC meeting, or for that matter who didn't happen to see this thread, would have no idea what was discussed, or by whom.
- This isn't some kinda of hairbrained scheme i hacked up in a few hours. I spent about 6 hours researching bmpman and the tcache code before even writing the first line of code
- Multiple people were involved in the discussion of how this would be best handled which include such high profile SCP and community members as The_E, Swifty, MjnMixael and chief1983
- The patch was code reviewed by swifty and the commit was greenlighted by chief1983
- Multiple people confirmed the patch working through various testing. Rodo, niffiwan, Spoon, MjnMixael, The_E and probably others.
These points pretty much address all of my my concerns.
5. Streaming single frames of explosion or impact animations instead of preloading them is bad. It should be rather obvious why. The question of why they are not done this way now is not a question of "this is only partially done" but a conscious decision in favor of performance.
Yeah, this makes sense. I thought this feature wasn't applicable to effects and textures, though, as Swifty mentioned
here? Even in the case of head anis, it is theoretically possible that a mod designer would use a single-frame head animation. Does the code compensate for this?
This isn't a hack. Just because I said that we were treating different kinds of resources with different use cases doesn't mean we're making some sort of unrobust and inelegant discriminatory code path.
Which is why I qualified my statement with a disclaimer.
I was trying to find out information, not throw out accusations.
I'm going to be less tactful and ask Goober when was the last time you submitted anything for code review? Cause the last time on the internal was in April, and if you're going to tell me that was the last time you committed something that could potentially break something, my response is going to be a rude noise.
In that time you've submitted quite a bit of code which I tend to feel you should at least have run past me as the other main FRED/SEXP coder. I doubt I'm the only coder who feels that you've submitted code you should have run past them.
So if you want us to take you seriously about code reviews, I'd like to see you posting them yourself before asking others.
Please don't try to turn this into a personal argument.
Valathil's SVN comment included the phrase "implement eff streaming", which is a feature that had been discussed multiple times in the past, in multiple threads, and had been deemed a fairly tricky problem with no easy solution. I don't demand a code review for every one of Valathil's commits, and I'm not trying to attack him personally. This is about "EFF streaming" as a feature, which is fairly a big deal, and as such warrants a fair bit of attention.
And based on all the responses to my question, it sounds like it got the attention it warranted. So I thank you all for bringing me up to speed.