07th-mod / higurashi-patch-compiler

Compiler for Higurashi patches. It's still under development.
7 stars 1 forks source link

Console (Sui/Switch/PS3) Background Music and Sound Effects (BGM and SE) #34

Open enumag opened 6 years ago

enumag commented 6 years ago

@DoctorDiablo @drojf @Grelo @idealpersona @InochiPM @irlPM @ItaloKnox @Norgus

(Wow the team has grown a lot. I hope I didn't forget anyone.)

With the LipSync releases mostly done (save for Meakashi which I'll look into next weekend) it's time to discuss the next release.

From what I know you guys are working on porting some interesting features from Sui. Let's discuss what features we should finish for the next patch and how to incorporate them:

  1. Sui video openings
  2. Sui background music
  3. Sui sound effects (?)

Sui video openings

So we have a hacked proof-of-concept DLL capable of playing the videos (thanks @drojf) but it needs more work, right? Can you guys share more details about the current status and how difficult it will be to finish it?

  1. When implemented in the DLL, how do we use the feature in the scripts?
  2. Are there any limitations or known issues?
  3. We'll need some article about how to implement the feature in new chapters which will be released later. Which DLL classes and functions are related, how the implementation works and what possible issues we should be aware of.

Sui background music

I know this was mostly done even before the lipsync release but we had to make a feature-freeze somewhere. As much as I'm looking forward to this feature I felt it deserved more attention than we could have given it with all the other features we were adding at the time. Mainly we need to discuss how to add it into the patch since there are several possible options which we should consider carefully. For this reason I rejected it. I apologize for making this decision pretty much alone. I do believe Italo would support me on this one though. Please let me know if you think it was a wrong decision or if I was too hasty with the rejection.

Anyway this allows us to make the next release kind of Sui-themed with adding both audio and video from Higurahi Sui. I'm already quite hyped about it.

From the little information I have about the Sui OST patch it's made automatically by a script written by @drojf.

Are there any known limitations / caveats for the scripts? Does it work for all chapters out of the box, including future ones (assuming the language in the scripts will be the same)? Also it needs the Sui XML - where do we have that? I have some testing versions but I don't think I have the latest.

I'm a bit confused about this from the sui merger readme: "Two merged scripts are generated - one which contains ALL instructions associated with each dialog, and one which has only the BGM play/stop inserted, and the old BGM commands removed." - what does "all instructions" mean in this context?

We also need to discuss how to add it to the game. I can think of several options:

  1. Have the commands for both original and Sui BGMs in one script using the GAltBGMflow P-Chang added for this purpose.
    • Pros: This would be the easiest for the users since the OST would be toggleable in the game.
    • Cons:
      • The script would be slightly more complicated than it is now.
      • The patches would be bigger.
  2. Make it an optional feature downloaded manually - separatly from the main patch.
    • Pros: Scripts and base patches would stay the same.
    • Cons:
      • We'd have to recomile the Sui variant for every release.
      • It would be more difficult for someone with Sui scripts to make pull requests.
      • Save files from one variant would be incompatible with the other.
      • Player can't easily switch between the OSTs.
  3. Previous option + choice in the installer.
    • Pros: Same.
    • Cons: Same + installation would be more complicated even when using the installer.

The option number 2 is the easiest to make now and suitable for a beta release in my opinion. This way it could even be released at the same time as stable lipsync patches if desired.

However if we want to really make this feature a part of our patches I'm personally more inclined to number 1 than number 3 due to the listed pros and cons. Did I forget any notable facts or other options?

Can you guys upload a beta patch with the Sui OST to this issue for testing?

Sui sound effects (?)

If we're adding Sui audios and videos it might be a good idea to also add Sui sound effects (the files in SE directory).

Is this a good idea in your opinion? Should we aim for addint these too?

I believe it should be possible to modify the Sui merger to add this as well?

irlPM commented 6 years ago

Regarding the video playback, it is working but the menu doesn't show up after playing a video I think, I'll be running some tests but mainly @idealpersona will be working on it after he is done with the Main release stuff.

.

DoctorDiablo commented 6 years ago

I'm not sure if the Sui sound effects would be an improvement. Some of them sound good, but I definitely prefer the original PC sound effects because they use significantly longer loops for the ambient noises (cicadas, crickets, rain, birds chirping, cars driving by, etc). They're 10-20 times longer in several cases, so the loops sound a lot more natural. Here's an example: Rain.zip (ame is from PS3, lsys is from PC) The engine also stutters a bit when it reaches the end of an audio file, which can be a problem when when the files are really short.

Actually, I wonder if it would be worth it to modify the console arcs to use some of the PC sound effects.

drojf commented 6 years ago

A week ago I re-wrote my python merger in C# (because it was very messy/hard to maintain). In doing so, I noticed I did some stuff which caused poor matching - the matching is now much better. Unfortunately I haven't documented it yet or put out any builds yet for this tool. It's currently at: https://github.com/07th-mod/cs-sui-merger

Here is an example script which used to have lots of problems, but now works OK: tsumi_025_3_merged.zip . Note that the large scale ps3 matching region (eg "please match mg script to the ps3 dialogue ids 100 to 5000) is manually specified - the merger can still run automatically, but it works better if you manually specify in some cases.

The parts like: <!-- [91823.0 > 115]: 「……わかりました。 --> is indicating that a fragment of a dialogue instruction was matched - my old script didn't know about dialogue fragments (dialogue separated by voices) so would be very confused sometimes. The format I used for printing above is [ <ps3_dialogue_id><fragment_id> > <mg_script_line_no>]: <fragment_text>.

I also generate a side-by-side "diff" for debugging, but to view it you need to open it in Notepad++, put one file on one side, the other file on the other side, then enable "synchronize vertical scrolling" side_by_side.zip

I'll try to at least add publish a release and add some more documentation later. The code base is still messy at the moment for this tool.

Edit: I can clearly see some parts where it seems to be one sentence off, for example:

//大石rvS09/11/170700452.「おそらく。kvS09/11/170700453.さぞやガソリン臭いだろうと思います」
    PlaySE(4, "S09/11/170700452", 256, 64);
    OutputLine(NULL, "「おそらく。",
            NULL, "", Line_WaitForInput); 
    PlaySE(4, "S09/11/170700453", 256, 64);
<?xml version="1.0" encoding="UTF-8"?>
<PS3_SECTION>  <!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~START~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<ins type="DIALOGUE" num="92180" dlgtype="1" data="大石rvS09/11/170700452.「おそらく。kvS09/11/170700453.さぞやガソリン臭いだろうと思います」"></ins>
<!-- [92180.0 > NULL]: 「おそらく。 -->
<!-- [92180.1 > 2860]: さぞやガソリン臭いだろうと思います」 -->
</PS3_SECTION> <!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~END~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
    OutputLine(NULL, "さぞやガソリン臭いだろうと思います」",
            NULL, "", Line_Normal); 
    ClearMessage();

    FadeOutBGM(2, 200, FALSE);

It just seems to be a bug, because the sentence fragments match exactly so it shouldn't have any problems matching it.

enumag commented 6 years ago

@drojf Nice progress!

Do you think it would be possible to use the matcher to add Sui sound effects as well?

What is the result of your script? Are you just removing all original PlayBGM calls and adding the Sui ones?

We'll need to update the game scripts to have both original and Sui PlayBGM calls but only use those that should be used according to player settings. P-chang already implemented some basic support ages ago, it should work like this:

if (GetGlobalFlag(GAltBGMflow) == 0) { PlayBGM( 2, "<original song>", 128, 0 ); }

if (GetGlobalFlag(GAltBGMflow) == 1) { PlayBGM( 2, "<ps3 song>", 128, 0 ); }

Is it good enough in your opinion? Can you update the script to do this?

drojf commented 6 years ago

it's a bit embarrassing to say but I didn't see your message until now...i forgot that some of the higurashi threads were important for me...

Do you think it would be possible to use the matcher to add Sui sound effects as well?

Assuming it's easily read/transformed from the XML file, yes. Any XML instruction can be added in.

What is the result of your script? Are you just removing all original PlayBGM calls and adding the Sui ones?

I was initially, but irlPM directed me to try and replace the existing ones. So now it tries to replace the existing playBGM/fadeBGM if they are close by. If it can't find one close by, it just inserts it act the current position. There was also some issue with the BGM channels, so uses the same BGM channel the script uses.

...Code snippet... Can you update the script to do this?

Yea, should be fairly easy. Is there any special formatting things I need to be aware of? Like is the newline after the if statement important?

enumag commented 6 years ago

Do you think it would be possible to use the matcher to add Sui sound effects as well?

Assuming it's easily read/transformed from the XML file, yes. Any XML instruction can be added in.

The problem with sound effects is that thair placement needs to be quite accurate. You previously expressed concerns about the accuracy but then made some improvements so... do you think it's accurate enough now to work well with sound effects?

Is there any special formatting things I need to be aware of? Like is the newline after the if statement important?

As far as I'm aware, no special formatting is necessary.

drojf commented 6 years ago

hmm...well, I still haven't fixed that one bug I wrote about earlier, which means it could be a sentence or two out, which could possibly mess up the timing for some sound effects. I'll see if I can fix that, and give you a reply later.

enumag commented 6 years ago

ping @drojf 😉

drojf commented 6 years ago

Sorry, haven't been working on it, but since beta version of Umineko Q 1080p is more or less done I should be able to work on it now

drojf commented 6 years ago

Changes

if (GetGlobalFlag(GAltBGMflow) == 1) {  PlayBGM( 2, "HM02_12", 128, 0 ); }
if (GetGlobalFlag(GAltBGMflow) == 0) {  PlayBGM(2, "hm02_12", 128, 0); }

Note that currently, the inserted PS3 Play and Fade BGM try to match up with the Play and Fade BGM calls in the original MG script. This can be disabled if you want, to just insert exactly where they are in the PS3 script.

//Below Instruction is from PS3 XML:
    PlaySE(3, "Kami", 256, 64);

I've attached the output when merging tsumi below. tsumi_025_3_debug_and_output.zip

enumag commented 5 years ago

I now fallback on using ellipses/punctuation if there is not enough stuff to match with.

Beware that the number of ellipses may differ between our version and PS3. Better not rely on the exact number of ellipses.

Note that currently, the inserted PS3 Play and Fade BGM try to match up with the Play and Fade BGM calls in the original MG script. This can be disabled if you want, to just insert exactly where they are in the PS3 script.

Hmm... I'm not sure. What do you think @orian34? You have the most experience with comparing our version with PS3 so you're the best one to answer this I think.

Sound effects are output as below. The comment is just for debugging so you can spot them in the game script - can remove later. I just use the template PlaySE(3, [FILENAME], 256, 64);, I don't use the channels, volume, or panning.

Maybe we should use the same conditions as for BGM but with GAltSEflow? I believe that was the original intention. There is no reason to have both original and PS3 SEs active at the same time, right?


What file did you use as base btw? I mean Tsumi doesn't have any _tsumi_025_3.txt. Also there seem to be outdated instructions for voices for some reason - PlaySE(4, "S09/19/173100074", 256, 64); instead of ModPlayVoice.

Can we see something from Oni?

orian34 commented 5 years ago

I try to do my best to keep a consistency but sometimes MG uses a different music, or I can't find a fitting equivalent for a PS3 music, or even the same music in PS3 has 2 MG musics because of the slight variations for the scenes that fit better.

Anyway, all that to say that PS3 and MG scripts have inconsistent bgm mirroring(to my despair). And that having a switch between PS3 and MG bgm will require some hand fixing to have it stay consistent.

drojf commented 5 years ago

Beware that the number of ellipses may differ between our version and PS3. Better not rely on the exact number of ellipses.

It uses punctuation ONLY if the line has too few japanese characters to match properly. Most of the time it just ignores punctuation

What file did you use as base btw? I mean Tsumi doesn't have any _tsumi_025_3.txt. Also there seem to be outdated instructions for voices for some reason - PlaySE(4, "S09/19/173100074", 256, 64); instead of ModPlayVoice.

I don't know, I just had it on my computer :S.

Can we see something from Oni?

I tried the scripts in oni, and most of them didn't convert correctly without manually specifying which region of the PS3 to match to (the program is meant to 'aid' you in selecting the region, but it doesn't work all that well yet). I only tried specifying the region for onik_005 as a trial - see below:

Specifying Regions

I specified the region to match against as ps3_regions = [[22955, 23546]] for onik_005 (I think the start match region is a little off), and ran the program. If you search for the string 4414 -> 23398 in the .html debug file, you can see there's a large section in the ps3 script which isn't in the MG script - When inserting instructions I think my program won't take that into account and just insert a huge amount of instructions in that region. My program does allow you select non-contiguous PS3 regions, but i'm not sure what you want to do about those bits.

BGMPlay Detection/BGM Channels

For this script, the if (GetGlobalFlag(GAltBGMflow) == 0) { } wrapper is broken. I had some code in there to detect the BGM channel as sometimes it's different for each script, but I lost my BGM files so the detection doesn't work. In this case the channel is 2. I think I added this in because sometimes playBGM is used for playing looping sfx? In this case the BGM channel is 1, so it doesn't correctly wrap the PlayBGM calls in if (GetGlobalFlag(GAltBGMflow) == 0) { }

You can download the output here: onik_005_22955-23546.zip

Also it needs the Sui XML - where do we have that?

check the pinned message in the 'special' discord channel

drojf commented 5 years ago

onik_009 seems to match quite well.

onik_009.zip

I think I just need to add back in code I had to search for the start/end of the MG script in the PS3 instructions.

enumag commented 5 years ago

I commited it here to see the diff: https://github.com/07th-mod/onikakushi/commit/1abfe97f99a6bbc980b9c3817261b416ec6328ad

Here are some changes we'll need:

1. Don't remove all the lines with GADVMode conditions.

2. Conditions with GAltBGMflow are not correctly indented. For example the very first line:

current output:

if (GetGlobalFlag(GAltBGMflow) == 0) {  FadeOutBGM( 2, 1000, TRUE ); }

correct output (tab at the beginning, remove tab before FadeOutBGM):

    if (GetGlobalFlag(GAltBGMflow) == 0) { FadeOutBGM( 2, 1000, TRUE ); }

3. It'll be better for my compiler have the comment on the same line as the instructio:

current output:

//Below Instruction is from PS3 XML:
    PlaySE(3, "Interfon", 256, 64);

correct output:

    PlaySE(3, "Interfon", 256, 64); // inserted PS3 instruction

Questions:

Shouldn't all the previously existing PlayBGM and FadeOutBGM calls be wrapped by if (GetGlobalFlag(GAltBGMflow) == 0) {?

Is it intended for both the original and PS3 SEs to be played?

drojf commented 5 years ago

Don't remove all the lines with GADVMode conditions.

wow, that's a really bad bug. I'm not sure why that happens - will have to check it out.

Remaining TODOs

no problem, will do

Questions

Shouldn't all the previously existing PlayBGM and FadeOutBGM calls be wrapped by if (GetGlobalFlag(GAltBGMflow) == 0) {?

yes, it's supposed to happen. the reason this doesn't happen is because a while ago i added code to detect which bgm channel the script is using (by scanning the script for playBGM instructions, then checking how long the audio file is for those playBGM instructions). I lost the music files on my computer so that detection doesn't work, and it defaults to channel '2' which is the wrong channel.

Now, it's probably easier to just specify manually which channel is the BGM channel for each script file, so I should add that in (or is there some other way to check?).

Is it intended for both the original and PS3 SEs to be played?

I wasn't sure, so i left them both in. I'm guessing it's meant to be the same as the BGM? that is, only play ps3 in ps3 mode, and only play original in original mode?

enumag commented 5 years ago

Is it even necessary to detect/specify a channel? Why not just handle all existing PlayBGM commands regardless of channel?

I wasn't sure, so i left them both in. I'm guessing it's meant to be the same as the BGM? that is, only play ps3 in ps3 mode, and only play original in original mode?

Let's try that. As I wrote before, the GAltSEflow should be used here, so:

drojf commented 5 years ago

Is it even necessary to detect/specify a channel? Why not just handle all existing PlayBGM commands regardless of channel?

when I discussed with irlPM, I was under the impression that the playBGM is sometimes used to play sound effects (like the sound of wind blowing), which might not be how it is played in PS3.....as in, sound effects might not also be played with the BGM Player. This might lead to some MG sound effects which should be kept in being disabled.

If this is not the case, sure, i'll just treat them all the same.

enumag commented 5 years ago

BGMs and SEs are in separate directories so the audio file would also have to be misplaced. I don't think we have such cases in the game and if we find any later, we'll move them to the correct directory and fix the scripts accordingly. So yeah, treat them the same.

DoctorDiablo commented 5 years ago

BGM and SE work differently in Sui compared to the MG engine. PlaySE always plays an audio file from the SE directory once, and PlayBGM plays an audio file from the BGM directory in an infinite loop until FadeBGM is called (or another BGM is played on the same channel). So any audio you want to loop needs to be in the BGM directory and called with PlayBGM.

The difference is how looping sound effects (wind, crickets, ambient car noises, etc) are handled. The Sui xml has these defined using SFX_PLAY, but we have to use PlayBGM. You can tell the sfx is looping because the single_play attribute will be 0. There can sometimes be two of these playing at once, so the channels do matter. I used the sfx_channel attribute to differentiate. I mapped channel 4 in the xml to channel 1 in the scripts, and 7 in xml to 0 in the scripts.

You don't necessarily have to match this, but these are all the channels I used in the console arcs:

drojf commented 5 years ago

Thanks for the information DrDiablo, was very helpful

  1. Don't remove all the lines with GADVMode conditions.

Fixed accidentally removed lines

  1. Conditions with GAltBGMflow are not correctly indented. For example the very first line:

Indentation fixed

  1. It'll be better for my compiler have the comment on the same line as the instructio:

Comments now on end of ilnes

  1. Shouldn't all the previously existing PlayBGM and FadeOutBGM calls be wrapped by if (GetGlobalFlag(GAltBGMflow) == 0) {?

All inserted ps3 instructions should be wrapped. Likewise, all

from the original MG scirpt are also wrapped

  1. BGM/SE mapping

I have used drdiablo's mapping - looping SFX now play on channel 0-1 (from ps3 7-4). Also have now ported the 'fade channel' ps3 instructions, which become converted to fadeBGM instructions on channel 0-1.

Non-looping SFX should remain the same (on channel 3)

  1. PlayBGM/FadeBGM instructions insertion

Previously I would only take the last PS3 play or fade instruction of each chunk of PS3 instructions. Now I've removed that completely, and it just inserts all instructions present in that chunk that it recognizes. This was put in as a post-processing filter incase the matching was very bad, but I don't think it's needed anymore/should be fixed in another way. This is also kind of necessary now that there may be multiple fadeBGM instructions per chunk (for the looping SFX)

Still haven't added back in the 'start/end' of script helpr - that would probably need to be added to save some time when you're using the tool

Here is the updated script: onik_009_OUTPUT.txt

enumag commented 5 years ago

I committed the file to see the diff here: https://github.com/07th-mod/onikakushi/commit/eb2f3b4522d61eb29e8222d7ea83540afd72a7d1

There are two things I'm not sure about:

  1. The first instruction with GetGlobalFlag(GAltBGMflow) == 1 is a FadeOutBGM on line 91. This means we're most likely missing some BGM that should play at the beginning.

  2. For some reason if (GetGlobalFlag(GAltBGMflow) == 0) { FadeOutBGM(0,1000,FALSE); } is added at the end. Since it's == 0, it's not a PS3 instruction so why it was added? I don't think it's harmful but it seems useless.

  3. In order to not mix the files in BGM and SE directories, can you add a ps3/ directory prefix for all ps3 instructions? Also the comment is quite useless - we can recognize these instructions easily by the condition and with this by the prefix as well.

// current
if (GetGlobalFlag(GAltBGMflow) == 1) { PlaySE(3, "Kyupirn", 256, 64); }  // inserted PS3 instruction
// should be
if (GetGlobalFlag(GAltBGMflow) == 1) { PlaySE(3, "ps3/Kyupirn", 256, 64); }

Other than that I think it's fine. So next step is to make this work for other scripts. You said there was an issue with recognizing the boundaries of our scripts in the PS3 xml, right? I'm not quite sure how to do that. Do you have some plan?

drojf commented 5 years ago
  1. The first instruction with GetGlobalFlag(GAltBGMflow) == 1 is a FadeOutBGM on line 91. This means we're most likely missing some BGM that should play at the beginning.

I'll have to look into that, not sure what it is

  1. For some reason if (GetGlobalFlag(GAltBGMflow) == 0) { FadeOutBGM(0,1000,FALSE); } is added at the end.

That was a leftover from when I was working out things with irlPM. My program literally puts a FadeOutBGM at the end of the script no matter what. I don't remember why I added it in - I'll take it out for now.

  1. In order to not mix the files in BGM and SE directories, can you add a ps3/ directory prefix....

Will do. Will also remove the comment.


You said there was an issue with recognizing the boundaries of our scripts in the PS3 xml, right?

So, I tried doing the following:

This will give you the following output:

[TOML MISSING]: Start/End of [c:\tempsui2\input\tsumi_025_3.txt] not specified.
Will try best to do matching, but suggest manually inputting start and end.
------- Finding first 5 entries -------
MG : [21 -> NULL]:      OutputLine(NULL, "「興宮PSより全車へ。",
PS3: [91639.0 -> NULL]: 「興宮PSより全車へ。

MG : [24 -> NULL]:      OutputLine(NULL, "興宮PSより全車へ。",
PS3: [91639.0 -> NULL]: 「興宮PSより全車へ。

MG : [27 -> NULL]:      OutputLine(NULL, "雛見沢営林署にて篭城事件発生。",
PS3: [91639.2 -> NULL]: 雛見沢営林署にて篭城事件発生。

MG : [30 -> NULL]:      OutputLine(NULL, "人質多数。",
PS3: [91639.3 -> NULL]: 人質多数。

MG : [33 -> NULL]:      OutputLine(NULL, "全車は至急、雛見沢営林署へ向かわれたし」",
PS3: [91639.4 -> NULL]: 全車は至急、雛見沢営林署へ向かわれたし」

Best guess at start PS3 ID: 91639
------- Finding last 5 entries -------
MG : [5960 -> NULL]:    OutputLine(NULL, "「仕方ありませんわね。",
PS3: [92561.0 -> NULL]: 「仕方ありませんわね。

MG : [5963 -> NULL]:    OutputLine(NULL, "本気の私の前には、レナさんのトラップなんて問題にもならないことを証明して差し上げて見せましてよ…!」",
PS3: [92561.1 -> NULL]: 本気の私の前には、レナさんのトラップなんて問題にもならないことを証明して差し上げて見せましてよ…!」

MG : [5969 -> NULL]:    OutputLine(NULL, "「さぁもう一度聞くぜ…。",
PS3: [92562.0 -> NULL]: 「さぁもう一度聞くぜ…。

MG : [5972 -> NULL]:    OutputLine(NULL, "この部屋を爆発させるなら、時限発火装置をどこに仕掛ける…?」",
PS3: [92562.1 -> NULL]: この部屋を爆発させるなら、時限発火装置をどこに仕掛ける…?」

MG : [5979 -> NULL]:    OutputLine(NULL, "沙都子は、本当はまだ怖いくせにニヤリと笑うと、目を閉じて精神を集中させ始める…。",
PS3: [92563.0 -> NULL]: 沙都子は、本当はまだ怖いくせにニヤリと笑うと、目を閉じて精神を集中させ始める…。

Best guess at last PS3 ID: 92563
You can copy this into the conf.toml file

[[input]]
path = "tsumi_025_3.txt"
ps3_regions = [[91639, 92563]]

And likewise to find the end of the ps3 dialogue, repeat the above with the last 5 mangagamer lines, and then take the last ps3 dialogue match.

This seems to work OK so far, but I would like to run it on a couple more scripts to check it works properly.

enumag commented 5 years ago

Yeah, this seems like something that should work pretty well!

There is one more thing that might cause some problems and I'm unsure if you considered it: The forking scripts such as zonik_001_vm00_n01.txt. Do you know how they work or should I explain?

drojf commented 5 years ago

I haven't considered it...although, I do have the option of specifying several regions of the ps3 script per each mangagamer script. Eg you could say, the manga gamer script maps to regions [1010,1020], [1070,1080], [1040,1050] of the ps3 script. In that way the program might be able to handle forking scripts.

If the order of the forking script already matches the ps3 script, then no additional action needs to be taken (in theory...), as my program just operates on Mangagamer Dialogue, and ignores the other lines during matching (more or less). There might be some other problem that crops up, though - I will try the forking script later.

I'm not sure I understand where the 'choice' is made though - is it contained in that script file or is it in a different script file? (Where the users clicks a button to select between dialog000, dialog001, and dialog002?)

enumag commented 5 years ago

The choice is done in the main script based on the GCensor flag. For example: https://github.com/07th-mod/onikakushi/blob/master/Update/onik_001.txt#L1023-L1024

The ModCallScriptSection takes a filename and function name within that file that it should execute. These functions are usually short. In many cases such function only covers one line: https://github.com/07th-mod/onikakushi/blob/master/Update/zonik_001_vm00_n01.txt?ts=4#L98-L105

This could easily cause issues with matching these functions the same way you match the normal scripts - there may not be enough lines to match. So instead it might be better to take the ModCallScriptSection calls in the main script into account:

I'm not sure how difficult this would be, especially the "revert state" part. It might be easier to just run each main script twice with one execution taking the

if(GetGlobalFlag(GCensor) >= X){ModCallScriptSection(

calls and the second execution taking the

if(GetGlobalFlag(GCensor) <= X){ModCallScriptSection(

calls (note the >= vs <= difference). Don't depend on the X, this is different in each case. But there are always exactly two branches, one with >= and one with <=.

drojf commented 5 years ago

I partially added ModCallScriptSection. Right now it merges the sub scripts the (z-scripts) into the main script before doing the comparison, but it doesn't un-merge them at the end, so you'll see the sub scripts inline in the output.

I handle the merging by just assuming that the first fork in each fork section is the "least censored" one. I use the "//VoiceMatching" comment to detect when a section starts - If not all fork sections start with "//VoiceMatching" then it probably won't work correctly the way I did it - I can use a different way.

(Oh, I just re-read your comment about using the >= to check which fork is the important one, and that there are always exactly two branches. Currently it doesn't run the script twice to handle both forks, it just takes the first one. I guess I'll need to fix that later. The information you've given me is enough to resolve the questions I had, I think)

edit: changed to use the >= to detect which fork is which. but I still only use the uncensored fork at this stage.


In the attached files, the files with the merged BGM and sound effects are in output/[script_name]_OUTPUT.txt I would use WinMerge and enable the options->compare->general->ignore carriage return differences otherwise the diffs will be messed up. You can compare against the files in the pre_input folder.

Downloads: sui_script_matching_15-06-2019.zip

You'll also want to look at the output/[script_name]_side_by_side_debug.html to see the matching quality.

I can already see a problem with the non-looping SFX insertion, in _tsum_003_3_OUTPUT.txt around line 70 - the SFX one after the other won't work properly. I don't think my script would ever be able to handle that kind of thing automatically anyway...more like this is something to be aware of.


A note about manually finding the start and end - it looks like I already had added in a system which helps you find the start and end by donig a brute-force match on the first 10 lines of the mangagamer script against the PS3 script. So it only takes about 10-20 seconds for you to enter in the start/end for each script. In other words, this part won't take a huge amount of time.

enumag commented 5 years ago

edit: changed to use the >= to detect which fork is which. but I still only use the uncensored fork at this stage.

Shouldn't you use the censored fork instead? I mean that's the branch which is closer to PS3.

drojf commented 5 years ago

you're right...I was thinking that PS3 version has "more content", but in reality it's the censored one...gotta fix that lol.

drojf commented 5 years ago

Here are the new results where it always takes the second, more censored fork.

sui_script_matching_18-06-2019.zip.zip