#8585 closed patch
engine init/shutdown OSystem methods
| Reported by: | SF/knakos | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | Port: WinCE |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
purpose: The backends get a chance to init/de-init specifically for the engine being brought up/shutdown.
practical reasoning: The ce backend creates the audio device just before the engine starts, so that the mixing rate matches the ogg/mp3 playback rate, supposedly for performance reasons. The way it was currently being done, was that the SetWindowCaption OSystem method (!) was overloaded. By taking a look at RunGame, we see that it's one of the last OSystem calls before engine->init and run. The gameid has been set at this point, so the backend can check out what engine is coming up. This is tacky to say the least. Enclosing engine init+go with two system->engineinit/ done calls is much cleaner. I have no use for the engineDone but it's there for symmetry.
possible future use: this scheme could be useful when/ if we implement the quit+return to launcher capability.
In current svn for ce, I'm still using the windowcaption overload to cleanly build.
Ticket imported from: #1595026. Ticket imported from: patches/690.
Attachments (1)
Change History (11)
by , 17 years ago
comment:1 by , 17 years ago
I don't quite understand why this would be the way to solve the "problem". Before adding such hooks, I'd really like to understand better why the backends *really* need them, and whether there aren't better / cleaner solutions.
In particular, this here sounds more like a request for a more flexible approach when it comes to backend mixing rates. Right now, we assume this rate is fixed and won't change over time. Apparently this is too inflexible for some uses, so we might want to revise how we deal with this. That would IMO be the proper fix -- while adding a code to detect engine startups and then change the mixing rate based on that seems to me to be a hack/workaround for the actual problem.
comment:2 by , 17 years ago
To modify the main architectural construct is surely a non-trivial issue. Adding methods the backends do not need or use leads with certainty to code bloat. These specific methods are general in concept and I agree with you that they don't solve precisely the problem at hand for the ce port (and it is an actual problem as it took me on and off a week to discover this extreme hack - perhaps better comments would help in that respect). Further this lack of specialization (i.e. too general) for the proposed methods does not agree with me too much.
I acknowledge though that engine starts and stops are important events in the lifetime of the OSystem object, especially if/when quit to launcher functionality is added, and maybe they should be globally signalled; at least that's my rationale behind this.
Flexibly changing the mixing rate is not exactly a requirement on the part of the wince port. I suspect this only needs to be done as rarily as engine changes, but I guess a transaction infrastructure as in the gfx case could be a good addition. This still leaves me though with the actual triggering event, which should be in the place of the (proposed) engineInit call in RunGame. In my opinion, adding a method like startMixer to replace that, it seems not so natural (e.g. there should be a similar gfx call or whatever).
Then again, the ce port works again now, even with this hack, as it has been working for a few years I guess :-)
comment:3 by , 17 years ago
how about rejecting or lowering priority on this Fingolfin? This is clearly not the way you'd prefer to go about this.
comment:4 by , 17 years ago
| Status: | new → closed |
|---|
comment:5 by , 17 years ago
I finally added this in. I am still not completely fond of this approach, but right now I just see no good alternative to this. We'll see whether/how backends will make use of this feature...
comment:6 by , 17 years ago
Are you sure Max? I am tempted to revert this, especially if I'm the only one I'm using it and only for setting up the mixer. In fact, I vote for it not to be in.
comment:7 by , 17 years ago
Well, just as you wrote in your initial comment, these methods are complementary to already existing ones. It is not bad to have this functionality in place just for a case. Who knows what backend needs will be arisen in future.
comment:8 by , 17 years ago
Kostas,
the main motivation to add these in now was *not* mixer stuff (I still would like to see the mixer handled differently; see my personal TODO list on the Wiki). But rather this is intended for keymapping. Many backends still need to setup custom keymaps, and I believe that engineInit would the appropriate spot for that (see also my recent email to scummvm-devel). Even if we implement the keymapping ideas, we still might need this, since the more I look at the issue, the more I think we may have to keep at least *some* custom logic for keymapping (e.g. to deal with Indy3/4 keyboard fighting). Should I turn out to be wrong in this regard, we can just ditch this API again.
So, in other words, what convinced me more is the idea that "engine start/stop are important events".
comment:9 by , 17 years ago
OK guys, I respect your decision. I will try to put it to good use and factor in there any stray engine/game init-related code the backend currently has. Thank you both for the explanations.
comment:10 by , 5 years ago
| Component: | → Port: WinCE |
|---|
patch at trunk/scummvm