Announcment to All Who are Writing Patches

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

User avatar
belugas
OpenTTD Developer
OpenTTD Developer
Posts: 1507
Joined: 05 Apr 2005 01:48
Location: Deep down the deepest blue
Contact:

Announcment to All Who are Writing Patches

Post by 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 ;)
If you are not ready to work a bit for your ideas, it means they don't count much for you.
OpenTTD and Realism? Well... Here are a few thoughs on the matter.
He he he he
------------------------------------------------------------
Music from the Bloody Time Zones

User avatar
Digitalfox
Chief Executive
Chief Executive
Posts: 704
Joined: 28 Oct 2004 04:42
Location: Catch the Fox if you can... 15 years and counting!

Re: Announcment to All Who are Writing Patches

Post by Digitalfox »

I'm not a coder like you know belugas :wink: , 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?? :D

Keep the good work belugas :D

User avatar
belugas
OpenTTD Developer
OpenTTD Developer
Posts: 1507
Joined: 05 Apr 2005 01:48
Location: Deep down the deepest blue
Contact:

Re: Announcment to All Who are Writing Patches

Post by belugas »

Nope, not at this point...
Maybe in a remotely distant futur, but that far beside the point.
If you are not ready to work a bit for your ideas, it means they don't count much for you.
OpenTTD and Realism? Well... Here are a few thoughs on the matter.
He he he he
------------------------------------------------------------
Music from the Bloody Time Zones

richk67
Tycoon
Tycoon
Posts: 2363
Joined: 05 Jun 2003 16:21
Location: Up North
Contact:

Re: Announcment to All Who are Writing Patches

Post by 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.
OTTD NewGRF_ports. Add an airport design via newgrf.Superceded by Yexo's NewGrf Airports 2
Want to organise your trains? Try Routemarkers.
--- ==== --- === --- === ---
Firework Photography

User avatar
belugas
OpenTTD Developer
OpenTTD Developer
Posts: 1507
Joined: 05 Apr 2005 01:48
Location: Deep down the deepest blue
Contact:

Re: Announcment to All Who are Writing Patches

Post by 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
If you are not ready to work a bit for your ideas, it means they don't count much for you.
OpenTTD and Realism? Well... Here are a few thoughs on the matter.
He he he he
------------------------------------------------------------
Music from the Bloody Time Zones

User avatar
Bilbo
Tycoon
Tycoon
Posts: 1710
Joined: 06 Jun 2007 21:07
Location: Czech Republic

Re: Announcment to All Who are Writing Patches

Post by 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 ... )
If you need something, do it yourself or it will be never done.

My patches: Extra large maps (1048576 high, 1048576 wide) (FS#1059), Vehicle + Town + Industry console commands (FS#1060), few minor patches (FS#2820, FS#1521, FS#2837, FS#2843), AI debugging facility

Other: Very large ships NewGRF, Bilbo's multiplayer patch pack v5 (for OpenTTD 0.7.3)

User avatar
belugas
OpenTTD Developer
OpenTTD Developer
Posts: 1507
Joined: 05 Apr 2005 01:48
Location: Deep down the deepest blue
Contact:

Re: Announcment to All Who are Writing Patches

Post by 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 ;)
If you are not ready to work a bit for your ideas, it means they don't count much for you.
OpenTTD and Realism? Well... Here are a few thoughs on the matter.
He he he he
------------------------------------------------------------
Music from the Bloody Time Zones

User avatar
belugas
OpenTTD Developer
OpenTTD Developer
Posts: 1507
Joined: 05 Apr 2005 01:48
Location: Deep down the deepest blue
Contact:

Re: Announcment to All Who are Writing Patches

Post by belugas »

First step is already in trunk.
I'll let you "discover" the new members.
Comments are welcome.
If you are not ready to work a bit for your ideas, it means they don't count much for you.
OpenTTD and Realism? Well... Here are a few thoughs on the matter.
He he he he
------------------------------------------------------------
Music from the Bloody Time Zones

User avatar
Bilbo
Tycoon
Tycoon
Posts: 1710
Joined: 06 Jun 2007 21:07
Location: Czech Republic

Re: Announcment to All Who are Writing Patches

Post by Bilbo »

belugas wrote: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.
If you need something, do it yourself or it will be never done.

My patches: Extra large maps (1048576 high, 1048576 wide) (FS#1059), Vehicle + Town + Industry console commands (FS#1060), few minor patches (FS#2820, FS#1521, FS#2837, FS#2843), AI debugging facility

Other: Very large ships NewGRF, Bilbo's multiplayer patch pack v5 (for OpenTTD 0.7.3)

DaleStan
TTDPatch Developer
TTDPatch Developer
Posts: 10285
Joined: 18 Feb 2004 03:06
Contact:

Re: Announcment to All Who are Writing Patches

Post by DaleStan »

Bilbo wrote: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.
To get a good answer, ask a Smart Question. Similarly, if you want a bug fixed, write a Useful Bug Report. No TTDPatch crashlog? Then follow directions.
Projects: NFORenum (download) | PlaneSet (Website) | grfcodec (download) | grfdebug.log parser

User avatar
Bilbo
Tycoon
Tycoon
Posts: 1710
Joined: 06 Jun 2007 21:07
Location: Czech Republic

Re: Announcment to All Who are Writing Patches

Post by Bilbo »

DaleStan wrote: 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 :)
If you need something, do it yourself or it will be never done.

My patches: Extra large maps (1048576 high, 1048576 wide) (FS#1059), Vehicle + Town + Industry console commands (FS#1060), few minor patches (FS#2820, FS#1521, FS#2837, FS#2843), AI debugging facility

Other: Very large ships NewGRF, Bilbo's multiplayer patch pack v5 (for OpenTTD 0.7.3)

DaleStan
TTDPatch Developer
TTDPatch Developer
Posts: 10285
Joined: 18 Feb 2004 03:06
Contact:

Re: Announcment to All Who are Writing Patches

Post by DaleStan »

Bilbo wrote:"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.
To get a good answer, ask a Smart Question. Similarly, if you want a bug fixed, write a Useful Bug Report. No TTDPatch crashlog? Then follow directions.
Projects: NFORenum (download) | PlaneSet (Website) | grfcodec (download) | grfdebug.log parser

User avatar
Octopussy
Engineer
Engineer
Posts: 122
Joined: 01 May 2006 21:57

Re: Announcment to All Who are Writing Patches

Post by Octopussy »

belugas wrote: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. 8)

Frostregen
Transport Coordinator
Transport Coordinator
Posts: 340
Joined: 06 Feb 2006 23:58

Re: Announcment to All Who are Writing Patches

Post by 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

User avatar
belugas
OpenTTD Developer
OpenTTD Developer
Posts: 1507
Joined: 05 Apr 2005 01:48
Location: Deep down the deepest blue
Contact:

Re: Announcment to All Who are Writing Patches

Post by 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
If you are not ready to work a bit for your ideas, it means they don't count much for you.
OpenTTD and Realism? Well... Here are a few thoughs on the matter.
He he he he
------------------------------------------------------------
Music from the Bloody Time Zones

Frostregen
Transport Coordinator
Transport Coordinator
Posts: 340
Joined: 06 Feb 2006 23:58

Re: Announcment to All Who are Writing Patches

Post by 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 :)

User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: Announcment to All Who are Writing Patches

Post by cirdan »

belugas wrote: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
Transport Coordinator
Transport Coordinator
Posts: 340
Joined: 06 Feb 2006 23:58

Re: Announcment to All Who are Writing Patches

Post by 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.

User avatar
cirdan
Director
Director
Posts: 539
Joined: 07 Apr 2007 18:08

Re: Announcment to All Who are Writing Patches

Post by cirdan »

Frostregen wrote: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
TTDPatch Developer
TTDPatch Developer
Posts: 10285
Joined: 18 Feb 2004 03:06
Contact:

Re: Announcment to All Who are Writing Patches

Post by DaleStan »

Frostregen wrote: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.
cirdan wrote: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?
To get a good answer, ask a Smart Question. Similarly, if you want a bug fixed, write a Useful Bug Report. No TTDPatch crashlog? Then follow directions.
Projects: NFORenum (download) | PlaneSet (Website) | grfcodec (download) | grfdebug.log parser

Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 3 guests