#9213 closed patch
Support for MI1 and MI2 talkies (fan patches)
| Reported by: | SF/logicdeluxe | Owned by: | |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SCUMM |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
For the Ultimate Talkie Edition project, I made some minor modifications to support MIDI, voice acting and saving preferences in these games. See the project page for details: http://www.lucasforums.com/showthread.php?p=2750295#post2750295
Attached is a patch I made against ScummVM 1.3.0svn from Sep 27. Games with known MD5 aren't effected by this patch, but versions without a known MD5 have an unrestricted extra entry, so that fan patches can be supported in its entirety. Also the string saving feature is enabled for all SCUMM V5 games. I don't see a reason why not support it like native SCUMM did anyways.
Ticket imported from: #3076698. Ticket imported from: patches/1318.
Attachments (3)
Change History (24)
by , 14 years ago
| Attachment: | patch.diff added |
|---|
comment:1 by , 13 years ago
| Component: | Engine: SCUMM |
|---|
comment:3 by , 13 years ago
Interesting! So all the patch does is to (a) remove the GUI options, which will make the "speech" options available in the options dialog, and (b) modify the SO_LOAD_STRING / SO_SAVE_STRING opcodes?
Regarding (a), that would mean that the game would work even with a plain vanilla ScummVM after slightly modifying the .ini file, is that correct? (Well, except for the modified opcodes, I mean) As for "saving preferences", what exactly does that mean? I assume the this is why SO_SAVE_STRING is modified, but ... why? When does the game save preferences, and what preferences, and how does the user edit those?
comment:4 by , 13 years ago
I am reposting this from the patch forum (http://www.lucasforums.com/showpost.php?p=2750295&postcount=320) because I think it will answer your question:
“This minor modification was required for two reasons: - Better support the options menu, since orignal ScummVM assumes that the enhanced CD has neither MIDI nor voice acting. - Even worse, original ScummVM refuses to use the string save feature, unless it is an Indiana Jones game. I use this feature to save the preferences (Spiffy mode, narator mode and subtitle mode).”
I hope this helps
comment:5 by , 13 years ago
As for "saving preferences", I'll quote from my readme.txt here:
3.3 Preferences
- CTRL-a toggles narrator mode. - CTRL-p toggles Spiffy mode between "SE based VGA", "EGA scan" and "no closeup". - CTRL-q toggles subtitle mode between "keep original text" and "match voice acting". - Preferences are stored in monkey.cfg, which is loaded automatically whenever a new game is started.
Of course, the way the patch works, the "monkey.cfg" has a leading target name added to the file name, just like ScummVM handles it with indy points, allowing independent multiple installations. The following lines of my readme states, that also music volume is saved, but this is not relevant to ScummVM.
As for the GUI behavior, I don't know. The original MI1 script overrides the scummvm.ini setting for some reason (just like Loom CD does). I did change this behavior some time after I made this patch, thus there was no reason to test this. Native MIDI, on the other hand, is not available without this change.
comment:6 by , 13 years ago
Any news on this?
Two tiny notes from my side:
- The "Fan patch" entries should be moved to the bottom of the monkey / monkey2 target list instead where they are placed right now. And probably have some specific comment why they are required too. - The added lines in script_v5.cpp should use tabs for indentation instead of spaces.
comment:7 by , 13 years ago
people keep asking about wii/droid builds with this patch...
i can't judge if the opcode changes are ugly, are they? we could maybe use a game feature flag to protect those changes for only the new detection entries?
comment:8 by , 13 years ago
An updated patch against master is available here: https://github.com/Littleboy/scummvm/commit/bc5cfe19e969f8f9af4bfec83e68ca1bcb0d4963
I've added comments and restricted the SO_LOAD_STRING / SO_SAVE_STRING opcodes changes to only monkey 1 & 2
comment:9 by , 13 years ago
Attached is a different patch, which differs from the submitted patch in that it hard codes a fixed filename for the fan patch config file, and restricts it to MI & MI2. Hopefully, this commit works as intended. If not, let me know ASAP. (I don't have a Windows machine nor a Windows copy of MI:SE myself, so I can't test the patch myself).
The main reason for the change was my desire to make game data validation easier / less risky; with the original patch, a malicious set of game data files combined with a none-validating savegame manager could potentially read/write arbitrary files.
Of course ScummVM isn't really doing a great job at handling malicious game data at all. But that doesn't mean we should needlessly add another potentially issue. In fact I think we need to start thinking about making ScummVM more resilient against bad data. But I am digressing. :)
Anyway, I am still rather reluctant to commit this. Because if I do, we are actively supporting the fan patch. The latest version of that I saw (RC 4), however, looked questionable from a legal point of view. E.g. it violates the GPL by including a binary ScummVM .exe, modified from the original sources, but not the source code, nor proper instructions as to how to obtain the source code. The only relevant remark in the "readme.txt" I could find was "You should find the modified source on the same site where you got this patch.". But this is clearly *not* enough to satisfy the GPL. E.g. what if I got that file from a friend? And in fact, I got the file from mediafire, and there sure is no way to find the patch from that. Even if it was, then a patch is still not quite enough, you also need to tell us where to get the rest of the code. No copy of the GPL was included either. I haven't even looked at the tons of other binaries included, but for some of them it also looks as if their license might be violated.
Lastly, it is not clear to me what the patch itself contains copyrighted data from LucasArts or other parties, as no source to that seems to be provided (or at least I couldn't find it). As such, I cannot evaluate whether this fan patch is "clean" or not. And hence I feel uncomfortable adding explicit support for it, at least until these doubts / concerns of mine have been resolved.
comment:10 by , 13 years ago
For possible violations, it was my intention to release the modified scummvm.exe in a separate file along with all the license texts in the next version. The complete modified source code is already available. In fact, I already did this separation with the MI2 patch WIP: http://www.lucasforums.com/showthread.php?t=205508 That strictly separates the ScummVM patch and the game patches, so there will be no ScummVM executable along with the Ultimate Talkie Edition patches. Which is no longer required when this patch gets officially applied anyway.
Of course, the patch to the game itself is not meant to be GPL, nor would it be possible due to the used codec. There is no way around a certain directx tool, namely xWMAEncode.exe. It is included for convenience with a link to where you can get it for free at Microsoft. I would remove that file, if it would help to satisfy your legal needs.
comment:11 by , 13 years ago
It is good to hear that you plan to address these licensing issues. And before I go on, I would like to point out that I never believed that you had any bad intentions, to the contrary. And I think it's quite admirable that you put so much time and effort into this work.
Having said this, all good intentions on your part and admiration on my part do not prevent legal issues from affecting us negatively. Since ScummVM has had some bad experiences with in particular LucasArts (no matter how unfounded their accusations were), we (or at least me) prefer to play it safe. No offense to you intended, though.
Now, regarding scummvm.exe being shipped in your fanpacth: Don't get me wrong, I know that you published the full patch for that here; and as soon as you start including the GPL text with your distribution and comply with the GPL's requirements,I am perfectly happy with that part :).
However, you got to understand this: When I notice some things being obviously not completely "clean", then this doesn't really encourage me to trust in the rest, for which it is less "obvious" to me whether licenses etc. have been complied with. And when I then notice even more "strange" things, I become wary.
Of course for a lot of what you bundle, one can relatively easily verify the origin and licensing term. For example, bsdiff is easily googable, and you even provide the homepage URL in your README; and it seems to be unmodified, too, so I guess it is OK (but IANAL).
But this is different for other things. ScummVM itself, for example (the "You should find the modified source on the same site where you got this patch." is not sufficient, as you can't know where I got the README from. Make this a fixed URL, at the very least to the scummvm homepage and this patch tracker item). Yeah, the issue will go away once / if we include your changes in our code base. Still...
But what about the the license and origin of unwx, miseAudioProcessing.cmd, extract_classic, ... ? For example, for "extract_classic" you write that it is a "simplified version of extractpak". IMHO this iis not enough -- unless the mysterious "extractpak" is public domain, I guess ? Otherwise, where does one get it from, what license is it under, what are the small changes? And there are several more files where it is completely unclear where they come from, as neither google nor your README seem to explain that (e.g. "build_monster.exe"), but for the benefit of doubt I'll just assume you wrote them yourself.
Oh, add in that I obtained your fan patch after quite some searching from mediafire.com -- a website notoriously known for being loved by people trading warez of all kind. Don't get me wrong, of course I don't consider your patch warez -- but the close connection to a shady area of the internet, combined with the lack of an official "store front door" (a website for the patch) and all that, just adds to the overall impression in a negative way.
Anyway, that all left (and leaves) quite some uncertainty on my side. In particular regarding the actual patch itself. That is, the "patchrc" files. This is IMHO the big beef, as it is what touches the LucasArts files. Don't get me wrong: It is not the licensing of that binary blob (I really don't care under what license you want to distribute that). It is just that I can't look inside that thing, and don't know what is in it, and so I'll just have to trust you that it contains nothing that violates the copyright of LucasArts. Normally I'd just trust your word on this, but given the, uhm, "mess" with the rest of your distribution, I have some reservations...
Put in short: Considering the overall impression, I am reluctant to officially condone your fan patch at this point. But I will be happy to reconsider as soon as things improve.
Some remarks on that: The license stuff is what seems most pressing to me. A proper homepage with a download link for the latest version, and the ScummVM.patch, would be another good step. lI could live with mediafire distritbution, although I find it user unfriendly (but maybe that's just me). For of course I am not the one who would have to pay hosting bills, so I can certainly understand it. But maybe some big site out there (the guys from the LucasArts Fan Network?) might be willing to help with both hosting of the files and a homepage for your wonderful fan patch.
comment:12 by , 13 years ago
i wanted to checkout that fanpatch too, but unfortunately its windows-exe-laden. but taking this a step further... if this can get it, we can also work on direct support for the classic version from the SE editions. which is in direct contrast to: http://sev-notes.blogspot.com/2009/07/steam-releases-vs-scummvm.html but such a solution would be way more appealing
comment:13 by , 13 years ago
I intend to add all the license texts from the tools used. For some it is indeed not easy to find any, though. unxwb, I find out it is gpl only by looking at the source code, which has this in the comments. I will mention this in the readme as well. ScummPacker has not really any mentioning of a license. I just asked jestar_jokin if he could add one, to clear this. miseAudioProcessing.cmd has no license mention either. It really is just some batch loops. Nothing special. I could just rewrite that part, if it makes any difference. extractpak has no license mention either. My modified source was damaged, unfortunately. I just asked bgbennyboy for a license text to clear this. For build_monster.exe etc. I will add them mentioned to the readme, that I wrote them on my own for the purpose of the Ultimate Talkie Editions, to make this clear too.
Of course, I do understand that the ScummVM team has to be cautions about this sort of things.
For the fixed link thing, while this tracker might be a good idea indeed, there is already a link to http://www.lucasforums.com/showthread.php?t=199288 where you also find the relevant links. Granted, it might be a bit confusing, as the initial post is terribly out of date. Espiox (the topic starter) wasn't there for a long time. He didn't answer to pm either. I asked, if an admin could update the initial post to reflect the current state.
I will also write a homepage dedicated to this patch and host all relevant files. Would the tracker link still be helpful in this case? Or should I just link the hosting homepage then?
comment:14 by , 13 years ago
Is it intentional that we do not error out anymore in the modified version of the patch on that opcode being used by targets other than indy4, monkey1, monkey2?
comment:15 by , 13 years ago
@lordhoto: that was a mistake. Thanks for pointing it out, will attach a corrected patch.
by , 13 years ago
| Attachment: | mi-fan.patch added |
|---|
Slightly revised patch against master branch (updated)
comment:16 by , 13 years ago
The latest patch by fingolfin doesn't allow you to change the subtitle/speech/both settings anymore. It's stuck on speech only for both Monkey Island games. Was this intentional or a mistake? Thanks!
comment:17 by , 13 years ago
I suspect that is because the fan translation entry is below the other entries. If we would move it to the top the settings would be usable for all other fallback detected versions too (I would *guess*). So I am not sure how this can be resolved properly.
comment:18 by , 13 years ago
| Status: | new → closed |
|---|
comment:19 by , 13 years ago
Support for these patches has now been added using a different approach in pull request #97, thus I'm closing this
comment:20 by , 13 years ago
Sorry, the final pull request for this is 99 (a rebased version of pull request 97)
comment:21 by , 5 years ago
| Component: | → Engine: SCUMM |
|---|
Modifications made for MI talkie projects.