AGS 3.3.x/3.4.0.0 Developer Discussion

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

Previous topic - Next topic

Crimson Wizard

#120
Quote from: Gurok on Sun 24/08/2014 07:50:00
https://github.com/gurok/ags/compare/adventuregamestudio:develop-3.3.1...331_runtime_type_information?expand=1
This is very nice, I am going to check this out today.

Quote from: Gurok on Sun 24/08/2014 07:50:00
The file format version for scripts was bumped earlier in the 3.3.1 commits, so compatibility-wise the new chunk of info in script files should be fine.
Just being curious, why was it bumped? I probably missed this.


Quote from: Gurok on Sun 24/08/2014 07:50:00
I think this is a small amount of double handling. Is there an argument for dumping the symbol table (and ALL flags about objects) into each script and reading it back? I think not because with the structure of the symbol table, this would mean a long scan of all symbols to get info about the pointers inside a struct.
Umm, I might have failed english or don't know something :-\. Can you elaborate what you mean?
Nevermind, I think I got this.

Crimson Wizard

#121
The issues I found:

1. RTTI should not be stored in Script objects, but in a global struct, or inside a global object. By "global" I mean not necessarily literally 'global' in C language, but the one persistent and accessible all the program time.
Why: the managed object must keep the reference to Type, - either a pointer, or numeric Id, - all the time until its deallocation. If it is a pointer - we can't tell if the script we took this reference from will be available when the managed object is destroyed. If it is a numeric id - the managed object class does not have any reference to current (or any) Script (and should not have, because its illogical), therefore it won't know where to take type info from.

Furthermore, the RTTI should probably be written to separate file in game package. It should include all types from all scripts (I guess this is what it does now?) E: I found it's not :(. I see that this will be more complex to do.

I suggest to introduce a new file to the game package, for example "script_rtti.dat", which would contain RTTI info from sym table in a simple format, e.g.:
- RTTI file signature ("AGS_SCRIPT_RTTI")
- RTTI format version (1?)
- the data
You would not need to increase game data version, because this is a separate file. Engine should just check whether it exists. If it doesn't, then scripts will work in "backwards-compatibility" mode, not allowing managed user objects, etc.

1b. You must not change how existing script commands and their arguments work, i.e. SCMD_NEWARRAY, because thus you will break compatibility with older scripts.

You may only do the compiler / serialization part. I can take care of the engine part after the compiler will be saving proper data.

2. Maybe we should start using std::vectors for expanding arrays, otherwise we will have to write "expand" function for every new type/array we put into program.
We are moving towards converting all the code to C++ anyway.

3. I have a thought that maybe we should allow to call "new" on any struct, even ones without "managed" keyword, because there's really no difference. What makes it managed is whether it is created on stack/global data or with "new" keyword. I would like to hear what others think about this.


EDIT: I investigated a bit more, and I see that the sym table actually stores only symbols from currently compiled module + the headers it includes. Something has to be done with this.
The problem is, that the object may be allocated in a room script, and then assigned to global variable. If we assign an Id of a type stored in the RTTI table of a room script, then we may loose this info after room was changed.
This means that the best way to achieve what we need is to keep a global sym table.

EDIT2: There's still a chance that, since module headers are always included in one order, the ids of types declared in upper headers will remain the same... meaning it can work, for starters.
Still this implementation would require managed object to lookup the "current script" for type info when it is about to deallocate, which is not good.

Gurok

#122
Quote from: Crimson Wizard on Sun 24/08/2014 13:41:38
The issues I found:

1. RTTI should not be stored in Script objects, but in a global struct, or inside a global object. By "global" I mean not necessarily literally 'global' in C language, but the one persistent and accessible all the program time.
Why: the managed object must keep the reference to Type, - either a pointer, or numeric Id, - all the time until its deallocation. If it is a pointer - we can't tell if the script we took this reference from will be available when the managed object is destroyed. If it is a numeric id - the managed object class does not have any reference to current (or any) Script (and should not have, because its illogical), therefore it won't know where to take type info from.

This is really hard, but I'll try. Copying the type information to the managed object when it's constructed would also solve the problem.

Quote from: Crimson Wizard on Sun 24/08/2014 13:41:38

Furthermore, the RTTI should probably be written to separate file in game package. It should include all types from all scripts (I guess this is what it does now?).
I suggest to introduce a new file to the game package, for example "script_rtti.dat", which would contain RTTI info from sym table in a simple format, e.g.:
- RTTI file signature ("AGS_SCRIPT_RTTI")
- RTTI format version (1?)
- the data
You would not need to increase game data version, because this is a separate file. Engine should just check whether it exists. If it doesn't, then scripts will work in "backwards-compatibility" mode, not allowing managed user objects, etc.


This feels a bit wrong to me, only because a game author would need to take this into account. e.g. He/she might need an alternate object management system just in case script_rtti.dat is deleted. I would prefer that we incorporate it into the main output file or add a flag when managed user objects are used so that the resultant game won't start without the script_rtti.dat file present.

Quote from: Crimson Wizard on Sun 24/08/2014 13:41:38

1b. You must not change how existing script commands and their arguments work, i.e. SCMD_NEWARRAY, because thus you will break compatibility with older scripts.


Oops. I take it I screwed up with the changes to read the size of the object. :-[ I'll fix that. Where/how should we embed the ID of a type in the compiled script then?

EDIT: My feeling was that we could pass the ID instead of the size to SCMD_NEWARRAY and SCMD_NEWUSEROBJECT, but use the file format to determine how it behaves (if < 90, treat it as size otherwise treat it as an ID)

Quote from: Crimson Wizard on Sun 24/08/2014 13:41:38

You may only do the compiler / serialization part. I can take care of the engine part after the compiler will be saving proper data.

2. Maybe we should start using std::vectors for expanding arrays, otherwise we will have to write "expand" function for every new type/array we put into program.
We are moving towards converting all the code to C++ anyway.


I can use those. No problem.

Quote from: Crimson Wizard on Sun 24/08/2014 13:41:38

3. I have a thought that maybe we should allow to call "new" on any struct, even ones without "managed" keyword, because there's really no difference. What makes it managed is whether it is created on stack/global data or with "new" keyword. I would like to hear what others think about this.


Ummm, that might get messy when people wonder why they can't return a struct created on the stack. Or wonder how to convert from stack variable -> heap variable. I kind of like the object management system the way it is. It's clunky, but it alleviates potential problem areas.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

#123
Quote from: Gurok on Sun 24/08/2014 14:04:32
Copying the type information to the managed object when it's constructed would also solve the problem.
That would, but that's a clumsy solution. It's like copying virtual table to every instance of a class instead of referencing one via pointer.

Quote from: Gurok on Sun 24/08/2014 14:04:32
Quote from: Crimson Wizard on Sun 24/08/2014 13:41:38
Furthermore, the RTTI should probably be written to separate file in game package. It should include all types from all scripts (I guess this is what it does now?).
I suggest to introduce a new file to the game package, for example "script_rtti.dat", which would contain RTTI info from sym table in a simple format, e.g.:
- RTTI file signature ("AGS_SCRIPT_RTTI")
- RTTI format version (1?)
- the data
You would not need to increase game data version, because this is a separate file. Engine should just check whether it exists. If it doesn't, then scripts will work in "backwards-compatibility" mode, not allowing managed user objects, etc.

This feels a bit wrong to me, only because a game author would need to take this into account. e.g. He/she might need an alternate object management system just in case script_rtti.dat is deleted. I would prefer that we incorporate it into the main output file or add a flag when managed user objects are used so that the resultant game won't start without the script_rtti.dat file present.
No, no, you seem to misunderstand what I mean. The script_rtti.dat will be inside game package, as well as all the other files.
Every script is a separate file too, and deleting them would screw the game anyway.
Game author would not need to worry at all, because this file will be added to final package automatically.
"Game data version" only refers to main game data inside the game package (ac2game.dta) - the one that describes the game generally.

EDIT:
Quote from: Gurok on Sun 24/08/2014 14:04:32
EDIT: My feeling was that we could pass the ID instead of the size to SCMD_NEWARRAY and SCMD_NEWUSEROBJECT, but use the file format to determine how it behaves (if < 90, treat it as size otherwise treat it as an ID)
That's possible, but I was thinking that in the future we won't need to use SCMD_NEWARRAY at all, SCMD_NEWUSEROBJECT could take care of arrays too (with some more info maybe). In such case SCMD_NEWARRAY will be used only for older scripts.

Gurok

Quote from: Crimson Wizard on Sun 24/08/2014 13:41:38
Quote from: Gurok on Sun 24/08/2014 14:04:32
Quote from: Crimson Wizard on Sun 24/08/2014 13:41:38
Furthermore, the RTTI should probably be written to separate file in game package. It should include all types from all scripts (I guess this is what it does now?).
I suggest to introduce a new file to the game package, for example "script_rtti.dat", which would contain RTTI info from sym table in a simple format, e.g.:
- RTTI file signature ("AGS_SCRIPT_RTTI")
- RTTI format version (1?)
- the data
You would not need to increase game data version, because this is a separate file. Engine should just check whether it exists. If it doesn't, then scripts will work in "backwards-compatibility" mode, not allowing managed user objects, etc.

This feels a bit wrong to me, only because a game author would need to take this into account. e.g. He/she might need an alternate object management system just in case script_rtti.dat is deleted. I would prefer that we incorporate it into the main output file or add a flag when managed user objects are used so that the resultant game won't start without the script_rtti.dat file present.
No, no, you seem to misunderstand what I mean. The script_rtti.dat will be inside game package, as well as all the other files.
Every script is a separate file too, and deleting them would screw the game anyway.
Game author would not need to worry at all, because this file will be added to final package automatically.
"Game data version" only refers to main game data inside the game package (ac2game.dta) - the one that describes the game generally.


Okay, let me try building a global table instead and get a commit up when I think it's ready for review again. I have a better idea of what to do now.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

#125
I don't know all the details about how Sym Table is composed, but I can see it uses names as id. I guess you may need to add script name to type name, in case user declared two types with same names in two room scripts. Just a thought.

EDIT: I was thinking about this more... that might be actually not easy to work with global RTTI table, because the Editor does not recompile all scripts every time, neither keeps them in memory. Some idea is needed here.
One of the possible solutions is to keep RTTI tables in every script as you do now, but with type names (string ids). Engine might have a map of types (with string as a key) and add types to global table as scripts are loaded.
When user object is created, engine will map script own type index into global string key.
Not the perfect solution, but may work for starters.

Gurok

#126
I was just about to come in here and post about that, CW :D

We can't really do a global table because each room is a separate compilation unit. We could perhaps serialise the script-specific table as we are now, but read it back when producing final output (regardless of whether the room was compiled) in order to create a global table. Seems cumbersome and difficult to do. I much prefer the suggestion you made of using a string at runtime. Perhaps the struct name with a script name as a qualifier, e.g. GlobalScript::Weapon

One thing I'm not clear on. To make my proof of concept work, I substituted a type ID for the "size" parameter in SCMD_NEWUSEROBJECT. This allowed SCMD_NEWUSEROBJECT to correctly identify the type of the object it was creating. How do you propose we pass this information to SCMD_NEWUSEROBJECT when the primary type identifier is a string?

EDIT: Regarding my last question, would you be happy if we:
- Use the script-specific type ID only when an object is created
- At that point, create an entry in the global type table using the true name of the type (e.g. GlobalScript::Weapon) if it doesn't exist
- Have the destructor use the global type table to find the type info when deleting the object?
Because I think that could work. I hope that's what you were getting at and I'm not completely off track.

EDIT:

Quote from: Crimson Wizard on Sun 24/08/2014 11:57:24
Quote from: Gurok on Sun 24/08/2014 07:50:00
The file format version for scripts was bumped earlier in the 3.3.1 commits, so compatibility-wise the new chunk of info in script files should be fine.
Just being curious, why was it bumped? I probably missed this.

I think I bumped it when new commands (do, for, break, continue) were added. In retrospect, it was probably not a file format change, but it would have been weird to have an earlier interpreter open it.

Also, I need to look into how exported types are loaded when a script is compiled. It's my understanding that the script compiler loads all previously exported symbols and then compiles the script (making it possible for a type ID to refer to types declared in previous modules). I'm not sure if my new "rtype" field gets populated at that time, however.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

#127
Quote from: Gurok on Mon 25/08/2014 02:28:31
We can't really do a global table because each room is a separate compilation unit. We could perhaps serialise the script-specific table as we are now, but read it back when producing final output (regardless of whether the room was compiled) in order to create a global table. Seems cumbersome and difficult to do.
There's an alternative to keep RTTI in a project folder all the time (same as room CRMs are kept in project), and read it in memory before any compilation.
Update it when a script is compiled. Since type ids have script module tag in them, program will know which types to update / remove (if module was removed). Then save it back to file.

Quote from: Gurok on Mon 25/08/2014 02:28:31
One thing I'm not clear on. To make my proof of concept work, I substituted a type ID for the "size" parameter in SCMD_NEWUSEROBJECT. This allowed SCMD_NEWUSEROBJECT to correctly identify the type of the object it was creating. How do you propose we pass this information to SCMD_NEWUSEROBJECT when the primary type identifier is a string?
What I suggest is to use solution consistent with function binding. Function binding works this way: every script has a list of functions it uses, and to call them it puts their local index in the byte code. Engine uses this local index to look up in the local table and find function name, under which it is registered in global table.
Similarily, regardless of whether we have a global RTTI, or a table in every script module, the numeric id will mean the index in local types array. This will be array of either full type descriptions, or only their names.
In any case - use numeric id to acquire string id from module's array of types, then this string is used further to find persistent type description from global table.
If we keep type infos in the module, the global table must be filled on script load, to get the types prepared.

Quote from: Gurok on Mon 25/08/2014 02:28:31
EDIT: Regarding my last question, would you be happy if we:
- Use the script-specific type ID only when an object is created
You mean - does not use it for arrays?
I guess we would actually need them for arrays too, because arrays may store user objects. This is not allowed now, but if we make this, it may be.
However, my idea was to merge SCMD_NEWUSEROBJECT and SCMD_NEWARRAY at some point in the future. The SCMD_NEWUSEROBJECT could take two arguments: a type id, and number of elements. If second arg is > 1, then array is created.
In such case SCMD_NEWARRAY will remain for backwards-compatibility only.
The problem here is that we need to somehow distinct array of pointers to struct and array of structs. I guess this is where "managed" flag may come of use. But I'd rather think a bit more about this.

Crimson Wizard

@Gurok and others, unfortunately I was pretty busy lately, and will be even more busy in next 5-10 days on my job (we are having deadlines here too :)), so if you post new variant of RTTI feature I may not be responding for some time.

Telling this just in case.

Gurok

#129
Quote from: Crimson Wizard on Wed 27/08/2014 20:30:39
@Gurok and others, unfortunately I was pretty busy lately, and will be even more busy in next 5-10 days on my job (we are having deadlines here too :)), so if you post new variant of RTTI feature I may not be responding for some time.

Telling this just in case.

Not a problem, CW. We are all busy with other commitments and jobs. I find it hard to squeeze in time too.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

I have a proposal to rename our development branch to 3.4.0 now, because it has bigger features now (improvements to scripts mostly), and probably will have more, while X.X.N+ versions are meant for very minor improvements and fixes. I was supposed to release a new update to 3.3.0 containing restored letterboxed mode and few other hot-fixes, now I am thinking about actually calling it 3.3.1, and using 3.3.2 number if more hotfixes are required.
(I should have probably done this much earlier).

Are there any objections to this? If not I'll just create a new "develop-3.4.0" branch right where "develop-3.3.1" now is, and the latter may be safely deleted a bit later when everyone gets updated.

Gurok

Quote from: Crimson Wizard on Mon 08/09/2014 22:54:19
I have a proposal to rename our development branch to 3.4.0 now, because it has bigger features now (improvements to scripts mostly), and probably will have more, while X.X.N+ versions are meant for very minor improvements and fixes. I was supposed to release a new update to 3.3.0 containing restored letterboxed mode and few other hot-fixes, now I am thinking about actually calling it 3.3.1, and using 3.3.2 number if more hotfixes are required.
(I should have probably done this much earlier).

Are there any objections to this? If not I'll just create a new "develop-3.4.0" branch right where "develop-3.3.1" now is, and the latter may be safely deleted a bit later when everyone gets updated.

No objections from me! Makes a lot more sense than the 3.3.0 Hotfix N approach.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Gurok

#132
Crimson Wizard (et al),

Firstly, in the development branch, I've found a problem with the most recent commits for D3D9 front buffer / back buffer stuff. I have a game project with sprite 0 set to:

http://i.imgur.com/ADz2Dtm.png (don't ask why)

The editor fails to load it, complaining that it can't find sprite 0. If I try to run the game data file with the most recent engine, I get a similar error on startup. This doesn't occur when I go back to a commit before 67de2af. Haven't fully investigated it yet. (I'm not asking you to fix it! Just documenting it!)


Secondly, my current large project has too many scripts (according to AGS):



I have exceeded MAX_SCRIPT_MODULES. I couldn't believe the error when I saw it. I don't think I'm doing anything wrong. I've got 1 module per extension (e.g. Mouse, Character), and a few extra modules for custom things in "Library", 1 module for each inventory item, global object, character and GUI. Okay, so I'm a bit of a neat freak, but is that an unreasonable way to organise things? (Might be the wrong forum.)

I checked your earlier 3.4.0 branch and there's no mention of MAX_SCRIPT_MODULES as a limit. I think increasing this from 50 to 200 would be sufficient for my needs. We could look at removing the limit in the future.
I monitored memory usage. With max=50, my RAM usage is 30.7MB. With max=200, my RAM usage is 30.8MB. I don't think upping the limit is a huge concern.

Edit: If my request isn't unreasonable, here's a pull request for that script module limit: https://github.com/adventuregamestudio/ags/pull/179
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

It is better to remove the static limit at all, if it is possible to do without much hassle.
The biggest problem I met were room areas limit, that could not be safely removed without completely changing savegame format.

Crimson Wizard

Regarding sprite error, I can't reproduce it. Only reason I can think of is the mistake introduced in commit 67de2af and fixed with next commit 6fa6e4e.

Gurok

#135
I'm sorry, CW. I misdiagnosed the sprite cache problem earlier. Here's a pull request to fix the problem that prevented my game project from loading: https://github.com/adventuregamestudio/ags/pull/181

Edit: I can PM you my game project on request, but it's pretty big!

Edit 2: I just submitted the pull request for removing the 50 script limit. Ohhhhh yeah! https://www.youtube.com/watch?v=oKnwHAGXvlE
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

Ok, I will make a final check tomorrow. Today I am just too sleepy :tongue:.

BigMc

Quote from: Crimson Wizard on Mon 08/09/2014 22:54:19
I have a proposal to rename our development branch to 3.4.0 now, because it has bigger features now (improvements to scripts mostly), and probably will have more, while X.X.N+ versions are meant for very minor improvements and fixes. I was supposed to release a new update to 3.3.0 containing restored letterboxed mode and few other hot-fixes, now I am thinking about actually calling it 3.3.1, and using 3.3.2 number if more hotfixes are required.
(I should have probably done this much earlier).

I don't like these +hotfix version numbers. Why don't you increase the third number for every minor release? It's not like there are not enought numbers, after 3.3.9 there's still 3.3.10 and people don't really care if the changes are minor or very minor. Having a release hotfix-2, then update-1 and then hotfix-3 means the versions cannot be sorted. That makes every Linux package maintainer crazy and also other people won't know if update-1 or hotfix-3 is higher.

Crimson Wizard

Quote from: BigMc on Fri 19/09/2014 18:30:07
I don't like these +hotfix version numbers. Why don't you increase the third number for every minor release? It's not like there are not enought numbers, after 3.3.9 there's still 3.3.10 and people don't really care if the changes are minor or very minor. Having a release hotfix-2, then update-1 and then hotfix-3 means the versions cannot be sorted. That makes every Linux package maintainer crazy and also other people won't know if update-1 or hotfix-3 is higher.

This is true, I just got carried away with this. Sometimes I am getting fixated on using some "scheme" and cannot leave it.
And people around seem too shy to make me stop :tongue:.
I will call next hotfix 3.3.1, and put an explanation in the topic so that people know that it is not Gurok's 3.3.1, but a further update of 3.3.0.

BigMc

And while we're at it... Since you are releasing preview binary releases (alpha,beta,rc..), I think it would be a good idea to give them also proper version numbers. The approach that is most compatible with sortability of versions is to give odd numbers to development versions and even numbers to stable releases, e.g. call the development branch now 3.5 and let that turn into the stable release 3.6. This approach is for example used by Allegro and the Linux kernel.

SMF spam blocked by CleanTalk