AGS 3.3.0 Release Candidate

[Adeel]

Wow, Crimson Wizard. I never saw read you this much angry before ! I can totally understand why you got angry. But amidst your anger, these lines you said are CLASSIC:

I mean, it is no big deal when a programmer makes a mistake by overlooking. We do mistakes, we test, we bang ourselves on forehead and say "wtf I done", we fix errors.

These lines really made my day! 

[Crimson Wizard]

Well, at least Adeel had fun in this situation.

My apologizes again, I needed few days to calm down and think. As a result of thinking I started a new topic.

I want to mention few things about "Global speech animation delay support" feature. While I had no objections against a property (I guess it is pretty logical), but removing legacy setting completely is incorrect, in my opinion.
Leaving aside the problem with XML reader:
1) This setting was used when old projects (2.72) are imported. With this setting removed users will have to fix their game by setting the new property in script instead.
2) The old (< 3.1? )games with sierra-style speech always used a fixed animation delay (5). Now they will use game.talkanim_speed instead, which is probably set to 0 for all such games (the animation will run faster than intended).
3) I can't help thinking that it would be nicer to just rename this global setting, removing "legacy" word, and add a way to set talkanim_speed in Global Settings as well. This would allow average user to quickly set up his game's speech settings in properties, instead of script.
(the global animation delay option may hide when "Use *" is off, and appear when it is on, like some of the properties already do)

[monkey0506]

removing legacy setting completely is incorrect, in my opinion.

In retrospect, I agree with this, and you raise some good points. Except:

2) The old (< 3.1? )games with sierra-style speech always used a fixed animation delay (5). Now they will use game.talkanim_speed instead, which is probably set to 0 for all such games (the animation will run faster than intended).

This is not true. JJS implemented a "fix" because A Tale of Two Kingdoms was stupidly using this variable in the AGS game scripts but expecting it to do nothing. The engine has always allowed game.talkanim_speed to work for Sierra-style speech (check the 3.2.1 source!). Fracturing the engine code because of a bug in the user's game scripts is simply unacceptable. The default for game.talkanim_speed is 5, which is what JJS set as the return value for Sierra-style speech. The fact of the matter is that if the user was not explicitly setting this variable, then it would always return that value.

[Crimson Wizard]

This is not true. JJS implemented a "fix" because A Tale of Two Kingdoms was stupidly using this variable in the AGS game scripts but expecting it to do nothing.

I think I have a clue now (I hope it is true). If it used (set?) talkanim_speed, but the results remained the same, this means that talkanim_speed was ignored when the animation delay is calculated internally for sierra-style speech? But it is not ignored anymore in 3.2.1. If this is so, then there should be a fix not in the GetCharacterSpeechAnimationDelay(), but at places where it is used internally in the engine. Perhaps some other function should be introduced, that makes a backward-compatible runtime fix. Well, I replied the same on the github.
I'd rather suggest we make some tests with pre 3.1 games.

[Radiant]

This is not true. JJS implemented a "fix" because A Tale of Two Kingdoms was stupidly using this variable in the AGS game scripts but expecting it to do nothing.

I'm surprised I've never heard of that, but ATOTK runs in AGS2.7, not 3.X.

Looking back on my code, ATOTK does the exact same thing as you recommended for Heroine's Quest: set talkanim_speed to GetGameSpeed() divided by some constant, to ensure that portrait animation speed remains the same when overall game speed changes.

[monkey0506]

I don't play AGS games, so it's possible I don't know what I'm talking about. But at least in the latest branches, game.talkanim_speed is defaulted to 5, which wouldn't change unless the game was expressly changing it.

Now, as I'm getting more information about this, what it sounds like is happening is that some player reported that the speech animation speed was broken because it wasn't speeding up when the game speed was increased. If the speed returns a constant value of 5 then the speed would be variable based on the game speed. If, as Radiant says, the variable value of game.talkanim_speed was respected (and set accordingly as he's described), then the speech animation speed would remain constant, regardless of game speed.

I was accusing the wrong person because it made more sense in that case (I'll admit, and I'm sorry for being so brash). And I don't really have all the full details on why JJS implemented this "fix" (which per Radiant's inspection of the code, would actually break the desired behavior), so there's still the possibility that I'm missing something else. But it really seems that this was wrongly reported, by whoever it was that reported it.

That said, yes, it warrants further inspection.

[Crimson Wizard]

Monkey, I made a test, and it seems pretty clear now.

More info in github comments:
https://github.com/adventuregamestudio/ags/commit/0d6e8f67973a4508cf05e04d1e110f18c49b1e8c#commitcomment-4344864



EDIT:

Looking back on my code, ATOTK does the exact same thing as you recommended for Heroine's Quest: set talkanim_speed to GetGameSpeed() divided by some constant, to ensure that portrait animation speed remains the same when overall game speed changes.

Did that actually work in ATOTK? My test shows that in 2.72 Sierra speech does not use talkanim_speed at all.
I think that's the one confusing moment about this issue. I hope monkey_05_06 is correct in what he said above regarding possible bug report.
I will be able to test ATOTK in few hrs myself...

BTW, this seem to work in HQ3 private beta build that you gave me, it is built with 3.2.1, where talkanim_speed should already be in use by Sierra speech.
It probably will not work with 3.3.0 beta 8, because there's a mistake we are discussing now (it brings behavior back to 2.72 standards).

[Radiant]

Did that actually work in ATOTK?

No, but I never took it out.

By the way, the music jitter issues reported earlier do go away on an AGS build without multithreading (i.e. the most recent 3.3 beta).

[Crimson Wizard]

Did that actually work in ATOTK?

No, but I never took it out.

Alright, that explains it all then.

By the way, the music jitter issues reported earlier do go away on an AGS build without multithreading (i.e. the most recent 3.3 beta).

Ok, I guess, we should disable that by default, maybe leaving an option in cfg file to turn it on for individual game/system if wanted. AFAIK mobile ports already have such option.

[Adeel]

Well, at least Adeel had fun in this situation.

To be honest, these words defined my situation in those days and that's why I've tried to immortalize them in my signature...

[Radiant]

• Starting cutscene while already in a cutscene. I get the line number of the new cutscene, but it would be great to have the line number of the first one (because that's generally where the error is - a missing EndCutscene call). The same applies to NewRoom while a new room is already triggered in the same script cycle

I'm afraid this one doesn't work; it reports "previously started at line 0".

[Crimson Wizard]

• Starting cutscene while already in a cutscene. I get the line number of the new cutscene, but it would be great to have the line number of the first one (because that's generally where the error is - a missing EndCutscene call). The same applies to NewRoom while a new room is already triggered in the same script cycle

I'm afraid this one doesn't work; it reports "previously started at line 0".

Can you give more details, like where the StartCutscene calls are located?

[Radiant]

It turns out to be the room script for room 83, line number 175. The second one is room 20, line 481.

[Crimson Wizard]

Well, I tried this:
room1.asc:

Code: ags

function room_AfterFadeIn()
{ 
  StartCutscene(eSkipESCOnly);
  player.ChangeRoom(2);
}

room2.asc:

Code: ags

function room_AfterFadeIn()
{ 
  StartCutscene(eSkipESCOnly);
}

And that displays correct line number.
Maybe there's something else...

BTW, I seem to have forgotten other scripts may be called during cutscene. I'll see if I can add script name to the message.

[Crimson Wizard]

There's something I do not know; can anyone tell, if the LipSync working along with Voice? If yes, what happens if text runs out, but voice still playing?

[Crimson Wizard]

Radiant, I am afraid I will not be able to implement AnimationStopTimeMargin working for voice right now.
When AGS plays voice it does not load whole clip at a time, but reads it part by part (streaming), therefore the full clip's length is not known at the moment of play start.
There might be a solution to this, but this will require redesigning audio system (for example, storing some information on sound clip separately in the game data), which I would not want to do right now.

[Radiant]

Ok, I understand. Thank you for your time.

[Crimson Wizard]

Ok, I made a branch specially for 3.3.0 release.
I think we should activate, how people call it, "feature freeze policy" for this branch.
The only exception may be implementing automatic scaling option in winsetup... although that's not a new feature, but a continuation of existing one. BTW the one we have currently works normally only for window mode, that should be fixed.


If someone wants to continue developing new features: fork a separate branch either from "master" or "release-3.3.0" (e.g. "feature_mynewfeatures"), preferably in the private repository. On other hand, you may consider continuing from the "develop" branch... the only problem would be that it is currently way behind in the sense of bugfixes compared to 3.3.0, and need to be updated as soon as 3.3.0 is ok.

[monkey0506]

The Android engine is pretty badly bugged right now, with it silently crashing all the time. If that can be sorted out, then should that be included in the 3.3.0 release?

[Crimson Wizard]

The Android engine is pretty badly bugged right now, with it silently crashing all the time. If that can be sorted out, then should that be included in the 3.3.0 release?

Yes, this branch is for bug fixes.

BTW, it is important to know if all ports compile and run properly.