Announcment to All Who are Writing Patches

[belugas]

In the next few days, I will introduce some new members to struct Window.
These will in fact be all the widget control functions that were introduced a while ago ino rder to remove the 32 widget limit we had before. But installed as members of Window (i said that already), without the Window keyword (quite redundant) and of course without the Window *w parameter

Understandably, yet again changing the widget code is not a nice thing to do for all those patches that are using widget code.
So in order to ease the transition, I will introduce the new members first, keep the old functions available for a week, move the old code base of functions to the new members (if ever you need examples of how to do it, which i doubt), another week than removal of the old functions.

For those who remember cpp_gui, it's part of that branch. By the way, that branch as itself is more a laboratory for gui ideas than something that will be merged eventually. The change i'm going to make is nothing more than using some of the ideas developped so far by KUDr.

Hoping no one will get too much trouble out of it

[Digitalfox]

I'm not a coder like you know belugas , but since i read a lot of people complaining that widget system ( and the rest of the Gui code ) is hard to understand and modify, will this make it easy to understand and modify, for people who wants to create patch's and even for developers??

Keep the good work belugas

[belugas]

Nope, not at this point...
Maybe in a remotely distant futur, but that far beside the point.

[richk67]

Errr... OK. Can someone take a look at the NewGRF_ports branch when the change happens, as its go several significant changes to the airport placement gui. Im afraid Im very busy at work, and preparing miniature models for an exhibition, and wont have time to code for a while.

[belugas]

No problem Richk67, but i woulds appreciate if you could synch it, as i do not want to break it, while trying to do it myself.
I know you are buzy, it it wil prevent damages that neither you nor I would like to see happening...
thanks in advance

[Bilbo]

So my temporary solution using my patch "raise that limit from 32 to 64" (http://bugs.openttd.org/task/925) will become obsolete? Good. :)

Would that remove all "artificial" limits on number of opened windows or some limits still remain? (Except limit by physical memory, speed of CPu trying to update all the windows, etc ... )

[belugas]

Not with the stuff i'm about to add Bilbo. It is simply about changing from pure global functions to class members, nothing more.
Of course, on the long run, there are things inside cpp_gui that will get into trunk, among which, i think, the so called "artificial" limit.

But as you remember, your patch was already obsolete at the moment you decided to make it limited to 64. But it was a good sign that something had to be done

[belugas]

First step is already in trunk.
I'll let you "discover" the new members.
Comments are welcome.

[Bilbo]

Not with the stuff i'm about to add Bilbo. It is simply about changing from pure global functions to class members, nothing more.
Of course, on the long run, there are things inside cpp_gui that will get into trunk, among which, i think, the so called "artificial" limit.

But as you remember, your patch was already obsolete at the moment you decided to make it limited to 64. But it was a good sign that something had to be done ;)

This is because 64bit int have limited number of bits and wider data types were not trivially available. It was the best temporary fix I could get with changing just 5 lines of code. Making it really unlimited would require some restructuring .... my patch was not meant to be permanent fiz, just temporary till some stuff from cpp_gui that removes the limit at all makes it to the trunk.

[DaleStan]

This is because 64bit int have limited number of bits and wider data types were not trivially available.

std::bitset?

OK, so you would have to do more than 5 lines of code change, and adding a new element could require promoting the bitset to a wider one, but neither are the end of the world.

[Bilbo]

std::bitset?

This may work, but I am not sure how often is that variable examined and extending from "trivial" 64bit type (which can get in a register in 64bit architecture) to "less trivial" bitset may introduce some slowdown. I think better way would be to rewrite the GUI so that this bitset is no longer necessary and my fix was only temporary fix to raise number of available windows from about 25 to 57 (some of the bits are reserver for toolbar and such). 57 windows "should be enough for everybody" in the short term (Since resolutions wider > 2048 are not supported anyway :)

[DaleStan]

"trivial" 64bit type (which can get in a register in 64bit architecture) to "less trivial" bitset

Bitset, internally, is a static array of machine words (In my 32-bit dev environment, unsigned long). For other processors, I expect that it's the widest fast integer.

Yes, there is a lot of supporting code in there. But, for 32-bit, most of it is what the compiler would generate anyway when emulating int64 on a 32-bit processor, and, for 64-bit, with only one array entry, most of it should collapse into essentially trivial code, assuming a competent compiler and library author.

[Octopussy]

It is simply about changing from pure global functions to class members, nothing more.

So GUI will be recast with Object-oriented programming ??

Nice ! The more OOP is OTTD, easier is programing.

[Frostregen]

Could you move
SetWindowDirty(Window *w)

to
Window->SetDirty() ?

Looks a bit inconsistent as it is now.

If it is already done,
please ignore this post

Thx,
Frostregen

[belugas]

Well... it is true that it would be consistent but...
On the other side, there is a check to see if the given window is null.
If integrated, the same check will have to done everywhere the function is actually used.
so... nope

[Frostregen]

Hmm,

nearly all calls[0] to SetWindowDirty(w) use a

w->Something

right before.
If w would be null, it would crash anyway.



[0]
I found only 2 occasion where this does not apply,
but maybe I have overseen a bunch.



Btw:
Thx for announcing you changes to patchwriters

[cirdan]

Well... it is true that it would be consistent but...
On the other side, there is a check to see if the given window is null.
If integrated, the same check will have to done everywhere the function is actually used.
so... nope

Can't the check be kept within the (member) function, instead of moving it to the caller? The check should test the 'this' pointer, of course. I've just tried that in a small program and it seems to work, ie, you can call a member function on a null pointer and it won't segfault (until you attempt to access instance data, that is); if the function first tests the 'this' pointer and returns if it is null, then nothing happens.

On the other hand, this could be implementation-dependent, or an extension in gcc. I'm certainly no C++ expert.

[Frostregen]

It should be only valid if the member function is static.
But "this" should always be null then...

I don't think it is valid code, but i cannot find any reference to proove it.

[cirdan]

It should be only valid if the member function is static.
But "this" should always be null then...

I don't think it is valid code, but i cannot find any reference to proove it.

'this' should not exist in static functions. But the question remains open as to whether a null 'this' pointer (in a non-static member function) is valid C++ or just a gcc-ism.

EDIT: I've been googling a little and it appears that calling a member function on a null pointer is officially undefined behaviour. So we can't rely on this.

[DaleStan]

It should be only valid if the member function is static.
But "this" should always be null then...

No. "this" is undefined in a static function. Not null. Just as it's undefined in a non-member function.

I've been googling a little and it appears that calling a member function on a null pointer is officially undefined behaviour. So we can't rely on this.

"Officially undefined" it may be, but the code for MFC (with at least one "if (this == NULL)") strongly implies that it works in MSVC too. This means that what I consider to be the two major compilers will both do the expected thing when calling a (non-virtual!) member function on a null pointer.

Anyone own a copy of the C++ spec, and care to cite chapter and verse?