AGS 3.3.x/3.4.0.0 Developer Discussion

Started by Gurok, Sun 09/02/2014 08:45:13

Previous topic - Next topic

Gurok

Hello,

I've been tinkering around with the AGS source for the last week or so. I'm mostly concentrating on some bugs that affect me, but I think they'll also benefit the community at large. Under Crimson Wizard's advice, I've also started back/forward/sideporting some of the changes in the Draconian edition to a format suitable for mainline AGS. Here are some things I am going to try to get into AGS 3.3.1. I'd like your feedback, comments, criticism, etc. I would appreciate code reviews, but I'd also just like general advice like "that's not important" or "we wouldn't want x option to work that way". I may change the target version for these, rethink them a bit or just scrap them completely.


Vsync for Direct3D
https://github.com/gurok/ags/compare/vsync

This is something that Alan v.Drake implemented in his fork, but it was hard coded. The real problem with Direct3D and vsync is that it needs to be initialised before the game starts. Here's what I did to solve the problem:
There's now a new option in winsetup.exe called "vertical sync".
If it's checked, the game starts with vertical sync turned ON.
In DirectX 5 mode, the game can still toggle this setting with System.VSync.
In DirectX 9 mode, the System.VSync property becomes read-only.


Code Regions
https://github.com/gurok/ags/compare/code_regions

This is another thing I grabbed from Alan v.Drake's work. He added some directives to allow you to wrap sections of code arbitrarily, e.g.

#region
if(something)
{
...
}
#endregion

You could then fold the code to show or hide these regions. Useful for large, hard-to-maintain global scripts.


Software Sprite Scaling
https://github.com/gurok/ags/compare/software_sprite_scaling

This was a common complaint when I was reading through a thread by Dave Gilbert about releasing commercial games with AGS. In Direct3D mode, the sprite scaling uses the screen's resolution and not the game's. Nothing breaks immersion like pixels of different sizes. I thought about fixing the Direct3D implementation. The thing is, by the time sprites end up at the Direct3D, we have next to no information about what their "real" size should be. It turns out the code path for software rendering of sprites is still available to Direct3D, it's just not being used. This patch provides a new option in General Settings called:

Force software scaling and flipping

When set to true, it will tell AGS to use software scaling, regardless of whether Direct3D's "superior" hardware accelerated scaling is available.


For, Break, Continue
https://github.com/gurok/ags/compare/for_break_continue

This would be the first baby step towards making AGS a more C-like language. I read a thread a while ago where CJ mentioned that for() and do and things like that were just duplications of while(). I don't agree that we should be limited to only using while statements. There's something to be said for the readability that comes with a standard set of control statements. That's why I'm proposing three new commands:

for([initialisation]; [check]; [increment])
{
    ...
}

continue;

break;

This would be the first step toward a more expressive AGS scripting language. I know there have been efforts to introduce LUA in the past, but this is something that can work immediately. In the future, I would plan to introduce more statements, like switch() and do, as well as struct parameter passing.

Technically speaking, the jumps for all of these commands already existed in the AGS code. There's a slight modification (some two phase parsing) needed to parse the increment of a for loop, but nothing more than what happens when calculating the jump for a while loop. I also refactored the parsing of assignments (first part of a for loop) to allow for this change.

This is a change that I think would need some serious testing. Everything it generates is stuff other commands generate (JZ, JMP, MOV, LITTOREG, etc.), but it's possible to seriously screw up a compile if some of the code is wrong. The good news is, if there are bugs, you can just remove the for, continue and break statements from your code and everything should function as before.

In terms of efficiency, continue is very efficient if it eliminates a check e.g. if(continue == true). Break involves the same number of jumps as a normal loop termination. There was no way to eliminate the second jump required. The for loop should be about as efficient was writing out a similar while loop by hand, e.g. x = 0; while(x < 10) { ... x++; }.


Portrait Positioning
https://github.com/gurok/ags/compare/portrait_positioning

Here is my unanswered thread about this problem:

http://www.adventuregamestudio.co.uk/forums/index.php?topic=49927.0

The positions for character portraits are sticky because AGS has no idea of who was talking before the current character once the speaking character has actually changed. My solution was to implement a two object "stack" of actively speaking characters. That way, if a character speaks, moves and speaks again, AGS still knows to whom that character was speaking and can use the relative position of the character to determine which side the portrait should be on.

If the speaking character is the first character to speak when entering a room, AGS takes a guess at which character they're speaking to by grabbing the first character it can find. I thought this could be a bit smarter, so I made it assume the character speaking was talking to the player first (if applicable), then try any random other character in the room.

This is a bit of a personal bugbear as X-based positioning for character portraits is unusable for me right now.


Text Window GUI Padding
https://github.com/gurok/ags/compare/text_window_gui_padding

This is one of the first things I proposed to CW and it's looking like 3.3.1 will be the release this makes it into.

It's very simple. Currently, text window GUIs put an amount of padding around the text in them. It turns out that this padding was a hard coded 3 pixels. This change adds a new property called "Padding" to text window GUIs and allows you to adjust this value (default is 3). I can't believe there haven't been any other complaints about this. It basically makes it impossible to make tight speech windows.


--

I'm currently focused on porting the stuff from Alan v.Drake that's still relevant. I might add to this discussion when I've got a list of the new scripting commands he added. Some of them will need to be discussed. People tell me to check the bug tracker. I've had a look there, but I also need to pick and choose things I can actually do. A lot of those tasks seem enormous and I'm only just finding my feet in the AGS source.


P.S. CW, is this the sort of discussion you were after?
[img]http://7d4iqnx.gif;rWRLUuw.gi

Calin Leafshade

All looks excellent and extra kudos for providing actual implementation and not just whining like the rest of us.

I think the thing that almost everyone is most interested in is struct parameter passing.
My suggestion for actual object creation can be found here: http://www.adventuregamestudio.co.uk/forums/index.php?topic=46157.msg620208#msg620208

Gurok

Thanks. I'll take a look at it.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Snarky

Wow, great to get another coder involved!

These look like fine improvements. I personally really appreciate vsync and for-loops. (And yeah, if we could pass pointers to custom structs, that would simplify coding by about 1000% percent.)

I do wonder about this bit, though:

Quote from: Gurok on Sun 09/02/2014 08:45:13
Software Sprite Scaling
https://github.com/gurok/ags/compare/software_sprite_scaling

This was a common complaint when I was reading through a thread by Dave Gilbert about releasing commercial games with AGS. In Direct3D mode, the sprite scaling uses the screen's resolution and not the game's. Nothing breaks immersion like pixels of different sizes. I thought about fixing the Direct3D implementation. The thing is, by the time sprites end up at the Direct3D, we have next to no information about what their "real" size should be. It turns out the code path for software rendering of sprites is still available to Direct3D, it's just not being used. This patch provides a new option in General Settings called:

Force software scaling and flipping

When set to true, it will tell AGS to use software scaling, regardless of whether Direct3D's "superior" hardware accelerated scaling is available.

Is it not possible in D3D to simply "merge" the background + all the sprites and stuff onto a single texture in the game resolution, and then scale up that texture to the screen/window resolution, all with hardware acceleration? I don't know that much about 3D programming, but that seems like the "right" way to do it.

Calin Leafshade

Yea, I meant to mention this. Using software scaling is not the right solution.

A better solution would be to have a RenderTarget of the native game resolution, do *all* drawing to that and then draw that RT to the back buffer.
That way you can still utilise filtering of various kinds and it still make sense with regards to the native res.


Alan v.Drake

Well, my D3D driver handled scaling just like that. Are you doing this via software so you can apply non-shader filters afterwards ? I always wondered if it's faster to get a bitmap from the d3d surface and then feed it to the filter and back. Shaders would still be better tho.

- Alan

Gurok

Quote from: Alan v.Drake on Sun 09/02/2014 14:50:26
Well, my D3D driver handled scaling just like that. Are you doing this via software so you can apply non-shader filters afterwards ? I always wondered if it's faster to get a bitmap from the d3d surface and then feed it to the filter and back. Shaders would still be better tho.

- Alan


I'm doing this because I didn't know you'd implemented better scaling. If your D3D driver did scaling as Snarky described, then maybe it's better to kill off my proposal and browse through the Draconian fork instead.

I actually thought there were complications involved with rendering to 320x200 and then upscaling it. I'm pretty sure I've seen some AGS games that are 320x200 but have high resolution fonts. Don't know how they work, but that was part of my concern. Also, I did it this way because the patch was minimal and it was relatively easy to do so (not so good reasons, I know).

I have heard there are some problems with filters in direct 3D, so maybe that's another reason to allow software scaling. But at the moment, I'm getting the strong impression that I should just ditch the current effort and do it the "proper" way.

I am looking at passing structs, Snarky. No fancy class keyword or allocation on the heap, but passing them in seems doable.
[img]http://7d4iqnx.gif;rWRLUuw.gi

tzachs

Looking great!

For loops are a welcome addition, and text window padding especially has been a pain. While you're working in that area, maybe you can look into having the line & character spacing also configurable (not only in text windows, but in Say commands as well) which would also help make some fonts more readable.




Snarky

Quote from: Gurok on Sun 09/02/2014 15:04:09
I actually thought there were complications involved with rendering to 320x200 and then upscaling it. I'm pretty sure I've seen some AGS games that are 320x200 but have high resolution fonts. Don't know how they work, but that was part of my concern. Also, I did it this way because the patch was minimal and it was relatively easy to do so (not so good reasons, I know).

Yeah, in earlier versions of AGS it was possible to make a 320x200 game and then set it to run in 640x400, which made the fonts display in twice the resolution. Some games (notably the Blackwell titles) took advantage of this. That feature-slash-bug was removed in recent versions, though (3.0, maybe?).

There's been some talk about allowing mixed-resolution games in a more deliberate way. (Perhaps an option to render fonts in a separate layer in the native screen resolution? Or a way to define certain graphical elements as being pre-scaled...) CW seemed to think it would make the graphics rendering code very messy.

(If we're talking feature requests, by the way, it would be cool to have the option to place sprites and text at non-integer coordinates. That would make smooth scrolling look better. Also higher resolution on sprite transformations like scaling and rotation: only having whole-percent and whole-degree accuracy makes these operations pretty jerky.)

Crimson Wizard

I made "develop-3.3.1" branch in central repository, so any updates may be pushed there.

Quote from: Snarky on Sun 09/02/2014 16:11:15
(If we're talking feature requests, by the way, it would be cool to have the option to place sprites and text at non-integer coordinates.
I must remind (again) that in AGS everything is interwined and one problem is usually blocked by another. It is of utmost importance to find the blocking issues, plan and resolve them first.
This, for instance, will require changing Plugins API, because current one has inner data of many engine types exposed.
Also, - providing compatibility hacks for old games that change object coordinates by writing directly to memory address.


I also urge you (practically beg) not to get every idea in one pile (again), regardless of how cool and wanted it is.
My belief is that the milestones for the next major version should be defined first (considering blocking issues too), and the work continued in a more advanced "develop" branch. As for 3.3.1 version, it should receive only minor updates that do not require changing the code much.

Radiant

Quote from: Crimson Wizard on Sun 09/02/2014 17:04:50I also urge you (practically beg) not to get every idea in one pile (again), regardless of how cool and wanted it is.
That's a good point. How about, instead of placing six ideas in one thread, we open six separate tickets in the Bugs And Suggestions Tracker?

QuoteMy belief is that the milestones for the next major version should be defined first (considering blocking issues too), and the work continued in a more advanced "develop" branch. As for 3.3.1 version, it should receive only minor updates that do not require changing the code much.
I agree that's a good approach.

Crimson Wizard

#11
Gurok made few merge requests, namely for:
- Code regions
- Portrait positioning
- Text window gui padding
- Vsync

If I understand correctly, there were no objections against these? These changes seem pretty simple ones.
I really hope Gurok tested them, especially the ones with padding and portrait positions (that all alternate options still work properly). :)
But since we are talking about development branch, it won't be a big problem if there are bugs there. I may merge them right away, then we may have the first test build.



Additionally, I can merge the long waiting feature branch by Alan v. Drake with his new script commands (extended Character.ChangeRoom function, etc):
Code: ags

void Character.ChangeRoom(CharacterInfo *chaa, int room, int x, int y, CharacterDirection direction);
int Object.IsInteractionAvailable();
int Hotspot.IsInteractionAvailable();
int Character.IsInteractionAvailable();

(or pick out the code manually to make it cleaner and easier to merge)
It bothers me that IsInteractionAvailable() returns "int". Since the return value is not used in binding script to engine, maybe it will be possible to change it to "bool" and still have compatibility with Draconian games.

EDIT: hmm, after rechecking the old branch, I think it will be easier to remake all them, because there's some excessive code in there (like ChangeRoom function have an "old-style" duplicate).

Gurok

I really hope I tested them too!

Portrait positioning only applies to really specific cases. The code I changed is after a series of checks that go a bit like this:

if(using_sierra_style_portaits)
    if(not_using_full_screen_ones)
        if(using_portrait_x_positioning)
            ...

Text Window GUI padding applies to all text windows. I tested speech and Display("") windows thoroughly. Both with border graphics and without. Is there anything else?

For, continue and break is the one I'm most concerned about. For a while, I accidentally had my for loop behaving a bit like a "yield" statement. It would quit the function and the next time you called that function, it would resume from that point. Finally tracked it down to a missing continue statement in cc_parser. That's why I'm concerned about merging that branch for now (at least until I've had more time to test it).
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

#13
I found a bug in TextWindow padding feature: it does not set a default padding value when loading old projects into editor. This means that all previously created projects will get padding set to 0.
How to fix: assign default value with member declaration instead of base constructor:
Code: csharp
private int _padding = 3;

(remove the "_padding = 3;" in TextWindowGUI() constructor).

Remember that you can do "ammend commit" thing to fix already existing commits, and "force push" to update your remote branch (Never do that in master, because that may cause conflicts with other people's work, but temporary feature branches are OK).


EDIT: Hmm, I forgot another case: importing a 2.72 project. It is done in a separate function in agsnative.cpp. We need to test that too.

Gurok

#14
All done. Sorry. I honestly thought the base() function was handling that. :/

REPLY TO EDIT: I believe this is already covered in my changes to read_gui:

    if (GameGuiVersion < kGuiVersion_331)
      guiread[ee].padding = TEXTWINDOW_PADDING_DEFAULT;

But that's a good point. I haven't tested it against 2.72.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

2.72 import is broken in 3.3.0 :(. I already found what caused this, there will be a fix soon.

Gurok

Is there a convenient link to 2.72 so I can do testing against it in the future?
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

#17
Quote from: Gurok on Mon 10/02/2014 11:41:35
Is there a convenient link to 2.72 so I can do testing against it in the future?

All here: http://agsarchives.com/engine.html ("Other builds" list at the right side).


EDIT: tested with fixed 2.72 import, it's working and padding = 3 for text windows.

deltamatrix

Has anyone proposed closures or lambda expressions in the AGS language? Or at least the ability to pass function pointers like in C and execute it.
BAD WOLF - TORCHWOOD - MR SAXON - THE BEES ARE DISAPPEARING - PANDORICA - RIVER SONG

Calin Leafshade

I think thats a little beyond scope at the moment.

If you need that then I suggest the AGS Lua plugin.

SMF spam blocked by CleanTalk