Thoughts on refactoring and futher improving the engine

Started by Crimson Wizard, Wed 20/06/2012 09:46:17

Previous topic - Next topic

JJS

I think the length() function would be broken for cases like this:

Code: c
char buffer[100];
strcpy(buffer, "hello");


The actual length is 5 but the function would return 99, wouldn't it?
Ask me about AGS on PSP, Android and iOS! Source, Daily builds

Crimson Wizard

#21
Well, as being said
Quote from: monkey_05_06a char[] has to have a size defined
In other words, that would work only when compiler can determine array size on its own.
This would be useless for char pointers when it is unknown what do they point at.

Also, I am not sure, how does this relate to a string class? String class is usually supposed to store string length (at least I think it should).

I have this proposition: make a simple and robust string, vector, etc classes. I my opinion defining their interface is more important at the moment, for implementation could be optimized and extended in future if wanted, but interface changes will require rewriting lots of code which uses them.

I should underline, that, in my opinion, a problem of AGS code is not only that it is uses unsafe arrays and strings, but also that it usually makes most of operations with them in place (e.g. string copying), thus increasing code size and repeating same simple algorythms over and over again all around.

Snarky

Regarding strings, please please please keep in mind to make the API Unicode compatible (UTF-8, presumably), so that one day we can support more than 128 (or 256) characters.

In any case, aren't there string libraries out there already? Not much point reinventing that particular wheel, is there?

Crimson Wizard

#23
Quote from: Snarky on Sat 23/06/2012 15:58:19
Regarding strings, please please please keep in mind to make the API Unicode compatible (UTF-8, presumably), so that one day we can support more than 128 (or 256) characters.
It's good you mentioned that, it seems I forgot about this completely  :shocked: At the moment I don't know how much problematic would be to start using wide strings in the engine, especially in the light of script system and backwards-compatibility. We need to check that out.

Quote from: Snarky on Sat 23/06/2012 15:58:19
In any case, aren't there string libraries out there already? Not much point reinventing that particular wheel, is there?
There *may* be portability issues, plus std::string (as well as STL in general) generates a lot of code, as was mentioned in the thread monkey gave a link to.
But I am sure there should be a good free string library somewhere in the web... I am wondering if monkey is looking for one or wants to write that himself no matter what...

To be completely honest, string and array classes are not so difficult to write, if you aim for practical simplicity and not some uber-universal classes.

monkey0506

#24
Quote from: JJS on Sat 23/06/2012 14:11:50I think the length() function would be broken for cases like this:

Code: c
char buffer[100];
strcpy(buffer, "hello");


The actual length is 5 but the function would return 99, wouldn't it?

That's a fair point, but that's why we would need a proper string class to begin with. Cases like this should be avoided altogether. If it's completely unavoidable, we could force the array to decay into a pointer. Based on some preliminary testing, there is actually (although admittedly small on my particular system) some benefit behind the static length in the case of string literals. This obviously doesn't benefit if it's stored in a pointer first, or once it's actually stored into the string class, but if you construct a string object using a string literal then having a static size is more efficient than having to parse it, looking for the '\0'.

I'm not against using a pre-existing string class (there are a million of them), but it wouldn't hurt to take into consideration exactly what AGS itself needs, and consider just writing our own. As CW said, it's not that they're difficult to write. Besides that, writing our own would also help future-proof code against licensing issues, portability, and so forth.

UTF-8 in particular would be a good case to work with, because then we wouldn't need to worry about using "wide characters". That is, wchar_t is not very well defined from compiler to compiler or between different system architectures, so using the 8-bit UTF-8 would allow us to stick to regular char and still accommodate international characters. I'm sure we would need to make some special handling to get an entire UTF-8 character (which could be represented by anywhere from 1 up to 6 4* chars/bytes). *Still reading about the UTF-8 specification, originally it could handle characters up to 6 bytes, but that was changed in 2003, so the official specification will only take up to a maximum of 4 bytes. Of course with the way the specification marks the bytes, it would be easy to back-track to the head byte of any given char in the string.

Edit: I was just thinking about it, and if we use UTF-8 strings, then "Length" should probably represent the actual number of UTF-8 characters rather than the number of chars used to store them. Based on my readings, it is a misconception to think that allocating larger blocks is a worse alternative than having to reallocate every time the string is changed. If we did implement our own UTF-8 string class, I'm thinking that "Length" should probably represent the actual UTF-8 character count, something like "CharLength" or "CharCount" should represent the number of chars in use, and "Size" should represent the allocated size of the buffer. This would probably end up negating the proposed benefit I mentioned before with regard to string literals, as both the UTF-8 length and actual char count would both need to be calculated. Oh well, just trying to be the very best, like no one ever was. To make strings is my real test, UTF-8 is now my cause.

Also, I've been looking and I'm not aware of any well defined portable methods of casting a floating-point value (particularly doubles) into a string/char*/char[]/etc. I've come up with something that can pull up to 10 digits into an integer and return the given precision (to distinguish between say 0.5 and 0.05). It seems to be rather functional (better than other implementations I've seen in any case).

AGA

Okay, so the old bug tracker's info is now imported, as 'issues' in the new system.  The information isn't perfectly transferred, but the way CJ wrote the old bugtracker was silly.

I have set up:

Quote from: monkey_05_06 on Wed 20/06/2012 16:49:17
Dave Gilbert, m0ds, monkey_05_06, Wyz, JJS, Calin Leafshade, ProgZMax

as admins for the Adventure Game Studio project.  If at some point we want to make up different projects (AGS iOS, AGS Linux etc), please let me know.

Crimson Wizard

Quote from: AGA
If at some point we want to make up different projects (AGS iOS, AGS Linux etc), please let me know.
Should there be a differentiation between development branches?
If I understand this right, there are at least three:
SVN branch (3.2 something)
JJS branch with ports which is obviously far away from SVN
my branch with refactored engine, which is - so far - about same program as JJS's, but with additional risk factor.

Now, if people find bugs in one branch, they won't be necessarily applicable to another.
And yet there's a question about feature requests. Consider something was asked to be added to AGS (editor or engine), where should it be implemented? I think this should be discussed a bit further.

tzachs

My thoughts on this:
The official version is SVN Main, and so the tracker should track bugs regarding the main SVN branch, not the unofficial versions, and I'm not sure about the development branch, but I'm tending towards no to that as well.

Eventually we will want to merge those unofficial ports into the main trunk and release an official version (the sooner the better IMO), we wouldn't want to keep those branches alive, this could create chaos for the exact reasons you mentioned.

Crimson Wizard

#28
Quote from: tzachs on Mon 25/06/2012 19:44:10
Eventually we will want to merge those unofficial ports into the main trunk and release an official version (the sooner the better IMO), we wouldn't want to keep those branches alive, this could create chaos for the exact reasons you mentioned.
I must disagree on what you say about "chaos". I don't think the situation I mentioned will necessarily cause chaos; it won't if the criterias for feature implementation are defined (i.e. what is allowed to go where).

But what I would really want to know: every development has certain goal. Was there any goal defined for the next official version (aka SVN Main)? Was it reached? If not, who continues to work on that?
Same for dev 3.2.2. What were your goals? Were they reached?
I am asking because I can hardly understand what's going on there, or rather why nothing is going on there.

I would like to also hear others on this. But from my position, that is - based on what I see and know, - it would be more logical to follow this release pattern:
- test and polish dev 3.2.2, and release it as 3.2.2; if there are any features added or fixes to engine and/or editor they are to be also merged to JJS's branch afterwards;
- finish, test and polish JJS's branch and release it as, for example, 3.3 (we could call it "AGS portable" or something);
- merge everything from JJS's branch into refactored branch and continue to develop it; perhaps release it as some 3.3.1 or 3.4 as an "experimental" release first, before real serious changes are made.
But that's a mere suggestion. I am still lacking understanding of everyone's plans...

BigMc

If you want to stick to a repository on CJ's server and import stuff there before a release, ok. But please then install git on that server. If you have worked with both, you know that git is technically better and nobody should import stuff into svn anymore.

AGA



AGA


timofonic

Quote from: AGA on Mon 25/06/2012 22:34:41
It's certainly an option.

Would CJ agree on converting the SVN repository to Git? After all, most active devs are using it so merging/forking would be easier and such.

(a dump of the original visual source safe repository would be appreciated to integrate it and save the code progress, for historical and retrocompatibility research reasons...)

Joseph DiPerla

Joseph DiPerla--- http://www.adventurestockpile.com
Play my Star Wars MMORPG: http://sw-bfs.com
See my Fiverr page for translation and other services: https://www.fiverr.com/josephdiperla
Google Plus Adventure Community: https://plus.google.com/communities/116504865864458899575

Crimson Wizard

[breaking news]

SVN repository, Common and Engine folders
*.cpp files count: 37

"refactory" branch, Common, Compiler and Engine folders
*.cpp files count: 260

Dum-dee-dum  (roll) (roll) (roll)

[/breaking news][/silly mode off]

Crimson Wizard

#36
I am close to finishing the "clean up" stage of refactoring (may take few more days).

Meanwhile I wrote an experimental code, introducing new classes (check commit changes to know what's new):
https://github.com/adventuregamestudio/ags/tree/pre_advance

EDIT: Just in case, I'll mention important units I added:
Common: core/err, core/out, core/types, platform/file, platform/path, util/keyvaluetree, util/stream, util/string;
Engine: core/engine, core/engine_setup, util/cmdargs, util/inifile;

I made the commit early so that others could check that out and tell what they think. If there are any wrong things it is important that they are known before we actually start putting this into use.
I am mainly interested in hearing opinions on coding style and general ideas.
Please, remember that most of new things there are either raw drafts or placeholders. The code may compile (I am pretty sure it will), but don't try to run it (it will do nothing or crash, I haven't actually tried, since that does not make any sense yet).

Joseph DiPerla

Quote from: Crimson Wizard on Tue 10/07/2012 13:31:19
I am close to finishing the "clean up" stage of refactoring (may take few more days).

Meanwhile I wrote an experimental code, introducing new classes (check commit changes to know what's new):
https://github.com/adventuregamestudio/ags/tree/pre_advance
I made the commit early so that others could check that out and tell what they think. If there are any wrong things it is important that they are known before we actually start putting this into use.
I am mainly interested in hearing opinions on coding style and general ideas.
Please, remember that most of new things there are either raw drafts or placeholders. The code may compile (I am pretty sure it will), but don't try to run it (it will do nothing or crash, I haven't actually tried, since that does not make any sense yet).

:0

I am so glad there are guys like you and JJS helping out on AGS. Otherwise I would fear it would halt in development. GREAT JOB!
Joseph DiPerla--- http://www.adventurestockpile.com
Play my Star Wars MMORPG: http://sw-bfs.com
See my Fiverr page for translation and other services: https://www.fiverr.com/josephdiperla
Google Plus Adventure Community: https://plus.google.com/communities/116504865864458899575

Joseph DiPerla

I keep getting this error when compiling:

Code: AGS
1>c:\Joeys fun projects\agssource-preadvance\adventuregamestudio-ags-33da32f\Common\platform/file.h(33) : fatal error C1083: Cannot open include file: 'Common/util/string.h': No such file or directory


I made sure I updated the VC++ directories to the new common folder, however, it still says its missing. I look in the folder and its there! So I have no idea whats going on. Idea's?
Joseph DiPerla--- http://www.adventurestockpile.com
Play my Star Wars MMORPG: http://sw-bfs.com
See my Fiverr page for translation and other services: https://www.fiverr.com/josephdiperla
Google Plus Adventure Community: https://plus.google.com/communities/116504865864458899575

Crimson Wizard

Quote from: Joseph DiPerla on Tue 10/07/2012 22:55:37
I keep getting this error when compiling:

Code: AGS
1>c:\Joeys fun projects\agssource-preadvance\adventuregamestudio-ags-33da32f\Common\platform/file.h(33) : fatal error C1083: Cannot open include file: 'Common/util/string.h': No such file or directory


I don't think you should try to compile that branch right now, let alone run it. As I mentioned, this was made rather to display certain ideas on class system. "Refactory" branch should be buildable and runnable though.

However, I found I forgot to add new dependancies for the Release version... again :)
Simply add following to the project's include directories:
$(SolutionDir)\..
or build it as Debug.

SMF spam blocked by CleanTalk