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

Right, constructor is the most logical thing to add.

Regarding serialization, all managed objects work via same interface, so its only a matter of writing/reading memory properly. In the case of "user object" it simply copies N bytes.

Snarky

This is fantastic! I have all kinds of stuff I'd like to use this for.

Calin Leafshade

Thats fucking cool guys.

I will be releasing some modules for this!

I think a constructor would be great and a destructor might be cool (?)

static methods should already work right?
Do properties work? I forget what AGS calls them.. attributes i think.


Crimson Wizard

Properties fail to work for some reason (unresolved import with weird error message).
I suspect that compiler needs to handle special case there.

ddq

This is really awesome. Excellent work, guys, and keep it up!

monkey0506

Quote from: Crimson Wizard on Tue 01/07/2014 16:08:54Properties fail to work for some reason (unresolved import with weird error message).
I suspect that compiler needs to handle special case there.

Actually, they work fine (download my test case). Since you're working with a user-defined type, the user also has to define the accessor methods. I've used extender methods so the accessors don't show up as normal member methods, but it is apparently working! :) And just in case anyone hasn't seen it, the attribute tutorial in the wiki is probably a good place to start.

The one issue I did have was with a static method returning a new pointer to the user type. The compiler complained that the "Size of identifier does not match prototype":

Code: ags
managed struct UserObject
{
  import static UserObject* Create();
};

static UserObject* UserObject::Create() // apparently doesn't work! "Size of identifier does not match prototype"
{
}


Other than that, this is pretty useful and exciting stuff. Thanks for taking the time to look into it. :=

Gurok

#66
Well, I created a cleaned up branch for it:

https://github.com/gurok/ags/tree/331_new_user_struct

Just a minor tidy up and monkey_05_06, your example should now compile. Not sure if the way I've fixed the bug is correct though. Going to possibly revise it in the future.

It relates to forward declarations. They don't know the size of themselves. That's fine, but all pointer declarations should have a size of 4.

While testing, I made a pointer to a test struct with two ints and the size of its type was 8 (or 0 at the time of declaration). 8 is correct for the size when calling SCMD_NEWUSEROBJECT, but not the size of the pointer. It looks like we need an if() statement to see whether it's a pointer somewhere and change its size accordingly. I'll have to investigate.

You should now be able to create an array of your managed struct too.

I haven't implemented constructors, but I'll be adding them after I fully investigate this pointer issue. And possibly clean up some other areas.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

#67
May you please make separate commits for changes in Engine and Compiler? Because they are, in fact, separate programs.
(I know, I am a great formalist)
That patch I made could work as a commit on its own. It will even bear my name on it :=.
(if something will go wrong everyone will know who is responsible :tongue:)

Quote from: Gurok on Wed 02/07/2014 13:48:56
While testing, I made a pointer to a test struct with two ints and the size of its type was 8. I believe this should be 4. 8 is correct for the size when calling SCMD_NEWUSEROBJECT, but in all the cases I saw, it looked like pointers were always 4.
I think you are confusing the size of struct and size of "pointer" (managed handle)?

Gurok

Quote from: Crimson Wizard on Wed 02/07/2014 13:56:00
May you please make separate commits for changes in Engine and Compiler? Because they are, in fact, separate programs.
That patch I made could work as a commit on its own. It will even bear my name on it :=.
(if something will go wrong everyone will know who is responsible :tongue:)

Quote from: Gurok on Wed 02/07/2014 13:48:56
While testing, I made a pointer to a test struct with two ints and the size of its type was 8. I believe this should be 4. 8 is correct for the size when calling SCMD_NEWUSEROBJECT, but in all the cases I saw, it looked like pointers were always 4.
I think you are confusing the size of struct and size of "pointer" (managed handle)?

Oh right, yes, I'll fix up the commits soon. Sorry. I would actually be happy if you made a branch and I sent you patches to fix things.

Yes, maybe you're right. I just had a look at some of the other managed types (Character *, Object *) and they seem to report their struct's size too.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

Quote from: Gurok on Wed 02/07/2014 14:12:41I would actually be happy if you made a branch and I sent you patches to fix things.
You can make pull requests to separate feature branches too. Or, it is what you actually mean?
But this really does not matter who makes a branch.

monkey0506

Quote from: Gurok on Wed 02/07/2014 13:48:56monkey_05_06, your example should now compile.

It relates to forward declarations. They don't know the size of themselves. That's fine, but all pointer declarations should have a size of 4.

I can confirm that your fix does allow it to compile. The explanation also makes sense, as does the solution. :)

Quote from: Gurok on Wed 02/07/2014 13:48:56You should now be able to create an array of your managed struct too.

I can also confirm this is working, though it took me a second to realize that I'm now responsible for creating the objects in the array too (simple enough to handle with a "CreateArray" function). Now if only we could have dynamic arrays as struct members... (roll)

Quote from: Gurok on Wed 02/07/2014 13:48:56I haven't implemented constructors, but I'll be adding them after I fully investigate this pointer issue. And possibly clean up some other areas.

This will definitely be helpful as well. :)

Crimson Wizard

Updated patch for the engine support of user objects:
http://www.mediafire.com/download/bo3d6c6y6a5hexw/0001-Engine-managed-user-script-object-support.patch

BTW, allows user objects of 0 bytes to 2 GB size. There's a reason why I used signed integer as "size" type (explained in comments).
This means "own" size of struct. You can allocate more size by putting pointers to other managed objects inside the first one.

Crimson Wizard

#72
I just wanted to make a small notice; it may not be really important for other developers at the moment, so just FYI.

After considering this for a while, I made a decision to not use the "develop" branch as a base for the future release, but instead start extracting commits by thematical groups to other branches, like develop-3.3.1.
Why: "develop" branch contains lots of various changes, some of them are ready, others still in "alpha" stage and require some extra work to make them finished; and these changes were made in random order, that makes it impossible to make partial merges (e.g. with 3.3.1). Also, there are even such changes that are not needed anymore, and would be reverted anyway. Some things made in haste and fixed for numerous times, etc.

This branch is a result of a very bad planning from my side. Also, back then I could not predict how far will 3.3.0 and 3.3.1 versions evolve.

I made a full list of commits in "develop" and grouped them thematically. I can already see which ones could be useful for 3.3.1: primarily cleaned up (refactored) code and some utility functions. Thus at least some of that code will get into real use at last.
The difference between develop and develop-3.3.1
Commits by group

I know that there's another branch based on "develop", the "multi_room_edit" by monkey_05_06, but its commits can be extracted same way when time is right (or simply rebased, since he practically was editing only one file). Same goes for any other work made on top of "develop" by anyone other (if there is some).


Gurok

#73
There's now a pull request for the managed object support:

https://github.com/adventuregamestudio/ags/pull/165

I wasn't able to get constructors working in a satisfactory manner without making the code look like Frankenstein's monster. I will attempt this again in the future. I have reasoned that it's not crucial for 'version 1.0' of user-managed objects, and we have enough to test as it is. We can introduce constructors at a later date and enforce bracket notation (e.g. n = new MyStruct() ) only when a constructor is present in the struct.

Regarding the implementation of constructors, I welcome anyone else to try. To be done properly, I think it will need serious refactoring.
[img]http://7d4iqnx.gif;rWRLUuw.gi

Crimson Wizard

Quote from: Gurok on Sun 13/07/2014 06:05:47
I wasn't able to get constructors working in a satisfactory manner without making the code look like Frankenstein's monster.
<...>
Regarding the implementation of constructors, I welcome anyone else to try. To be done properly, I think it will need serious refactoring.
I +1 this. When I was testing monkey_05_06 addition that allows managed arrays in structs, I noticed that there's another piece of code that creates global/local arrays, it looked 98% the same, but was located in separate function.

Crimson Wizard

#75
Urgh... I just had a revelation that told me that user objects won't work :(.
Because they do not know how to remove reference count from managed objects inside them when they are disposed themselves.
I will check how common structs work, but I suspect this is done by compiler (when local or global struct is deallocated).

For dynamic arrays it's not a problem, because they store only one type, and know what it is exactly.

This problem is far more complex than anything else related to managed structs. I will need to think more about this.
E: The dumb way is to have a list of offsets for all managed handles inside struct. In any case this means that script interpreter must support some kind of "type description".

monkey0506

This may be a good time to port the compiler fully over to managed code (C#). I've actually started exploring that already, but I need to understand the compiler better to make a useful conversion. I've been going through and heavily commentating the script in a separate local branch. If anyone would find it useful I could share, once I'm finished documenting.

Snarky

I really don't mean this as discouragement or criticism, but I wonder if all the energy put into hacking the scripting language might not be more usefully spent on other parts of the engine. The fact is that AGS Script isn't that unique, and we might be better off simply adopting a standard one with an existing compiler than having you spend so much effort trying to improve what we have. We're unlikely to be able to do better than off-the-shelf in this area, I think. We've already discussed alternatives, from Lua to AngelScript or Javascript (or perhaps UnityScript?) to Mono/C#. Personally I feel this is the path we should take, and that exploring the different alternatives is more useful in the long run than figuring out how to reimplement the wheel.

But again, I'm not the one doing the work, so take this for what it's worth.

Crimson Wizard

@Snarky, that's the reason I, personally, don't really want to touch it much.
I'll see if the user objects can be made work without much hassle...

monkey0506

If AGScript is replaced by a Java-based language, I will stop contributing to AGS development.

SMF spam blocked by CleanTalk