Face customization patch - TESTING requested!

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

Bot_40
Engineer
Engineer
Posts: 105
Joined: 27 Jan 2005 18:56
Contact:

Post by Bot_40 »

White looks the best, the ones with many colours seem a bit 'random'. If this gets into the trunk I'll be well chuffed, this is a feature I always thought was greatly needed.
User avatar
jez
Traffic Manager
Traffic Manager
Posts: 158
Joined: 23 Aug 2003 21:24

Post by jez »

I've updated my original post, now it contains v2 of the patch.

The GUI has been rearranged, the attribute numbers are white, all now start at 1 instead of 0 and increment contiguously.
=== Jez ===
User avatar
jez
Traffic Manager
Traffic Manager
Posts: 158
Joined: 23 Aug 2003 21:24

Post by jez »

I think these 2 face variable structure files may be useful to code reviewers too. One's my interpretation of what the old (32 bit uint) structure was, and the other's how my new code interprets it.
Attachments
facevar_structure_new.txt
(722 Bytes) Downloaded 258 times
facevar_structure_old.txt
(945 Bytes) Downloaded 259 times
=== Jez ===
Loop
Engineer
Engineer
Posts: 72
Joined: 10 Apr 2006 21:05
Location: Poznan, Poland

Post by Loop »

Great job on this!

Can't wait to see it in the MiniIN or in the trunk. Excellent!

I have only one, little idea: why not turn around the Load and Save buttons, so save is on the left, and load on the right. Dunno why, it seems more natural to me like this.
User avatar
jez
Traffic Manager
Traffic Manager
Posts: 158
Joined: 23 Aug 2003 21:24

Post by jez »

In many GUI toolbars, 'open' (similar to Load) is to the left of 'save', so I think this way is more intuative.
=== Jez ===
Bjarni
Tycoon
Tycoon
Posts: 2088
Joined: 08 Mar 2004 13:10

Post by Bjarni »

I wonder if we will see a version 3 of this patch with the coding changes applied to it. After giving feedback on the code, I haven't heard anything :?

Will this patch be finished or is it one of those that will only make it half way. It can't be added to the trunk in it's current form because the code is really messy. For one, who knows what goes on where, it should not be hard to fix though.
User avatar
jez
Traffic Manager
Traffic Manager
Posts: 158
Joined: 23 Aug 2003 21:24

Post by jez »

Have edited the original post, which now contains v3 of the face customization patch.

It utilises the new widgets code and doesn't have to fake widgets any more, as well as having a plethora of code changes that make it more consistent with the OpenTTD coding style. The screenshot is unchanged from v2 as the GUI is virtually identical.
=== Jez ===
Loop
Engineer
Engineer
Posts: 72
Joined: 10 Apr 2006 21:05
Location: Poznan, Poland

Post by Loop »

Cool!

Will it be tested and included in the trunk? :D
DaleStan
TTDPatch Developer
TTDPatch Developer
Posts: 10285
Joined: 18 Feb 2004 03:06
Contact:

Post by DaleStan »

Well, only you can control whether or not you test it.
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
jez
Traffic Manager
Traffic Manager
Posts: 158
Joined: 23 Aug 2003 21:24

Post by jez »

Uploaded v4 of the patch. Fixed a minor argument type issue.
=== Jez ===
User avatar
jez
Traffic Manager
Traffic Manager
Posts: 158
Joined: 23 Aug 2003 21:24

Post by jez »

Uploaded v5 of the patch. Conforms better to OpenTTD coding conventions.
=== Jez ===
User avatar
glx
OpenTTD Developer
OpenTTD Developer
Posts: 622
Joined: 02 Dec 2005 15:43
Location: Drancy(93) - France
Contact:

Post by glx »

/* Eyes */ switch is not needed

Code: Select all

if (FC_GETEYES > face_limits[FC_GENDER_ETH_COUNT][FC_EYES])
  FC_SETEYES(face_limits[FC_GENDER_ETH_COUNT][FC_EYES]);
does the same

/* Suit / Collar */ switch is not needed

Code: Select all

/* eg. (x % 4 - 2) limits x to the 3rd value in any block of 4. */
if (thisval % 4 > 2) {thisval -= (thisval % 4 - 2);}
break;
is enough
User avatar
jez
Traffic Manager
Traffic Manager
Posts: 158
Joined: 23 Aug 2003 21:24

Post by jez »

Posted v6 of the patch, incorporated glx's comments.
=== Jez ===
User avatar
skidd13
OpenTTD Developer
OpenTTD Developer
Posts: 522
Joined: 03 Mar 2005 10:49
Location: Germany

Post by skidd13 »

Nice patch. :)
Attatched the german translation for the patch.
But STR_FACE_GENDERETH fits in german not in the widget. :(
Maybe anyone has a better translation?
Attachments
german.patch
(3.63 KiB) Downloaded 254 times
What does that mean - the circumstances? I determine what circumstances prevail. -- Napoleon Bonaparte
---
If we cannot end now our differences, at least we can help make the world safe for diversity. -- John F. Kennedy
---
Our problems are man-made, therefore they may be solved by man. No problem of human destiny is beyond human beings. -- John F. Kennedy
User avatar
glx
OpenTTD Developer
OpenTTD Developer
Posts: 622
Joined: 02 Dec 2005 15:43
Location: Drancy(93) - France
Contact:

Post by glx »

Never do translations for a patch, if the patch is added to trunk you'll need to redo it with WT2 :)
User avatar
jez
Traffic Manager
Traffic Manager
Posts: 158
Joined: 23 Aug 2003 21:24

Post by jez »

glx: no idea what you meant by that, but I've incorporated the German translation into v7 of my patch anyway. ;-)

v7 also widens the face window so that nasty long German language can fit in there :-P, and slightly modifies DrawFaceStringLabel() so that the labels' draw location is completely based on their associated widget.
=== Jez ===
User avatar
jez
Traffic Manager
Traffic Manager
Posts: 158
Joined: 23 Aug 2003 21:24

Post by jez »

skidd13: As an aside, I had to muck around a with with that .patch file before I could get it to work. What's up with it? It seems to be slightly corrupted; this is the second time I've had trouble getting someone's patch on here to work. What tool are you using to create it? TortoiseSVN certainly didn't like it. I initially got the error, "The chunk size did not match the number of added/removed lines!".
=== Jez ===
User avatar
skidd13
OpenTTD Developer
OpenTTD Developer
Posts: 522
Joined: 03 Mar 2005 10:49
Location: Germany

Post by skidd13 »

jez wrote:skidd13: As an aside, I had to muck around a with with that .patch file before I could get it to work. What's up with it? It seems to be slightly corrupted; this is the second time I've had trouble getting someone's patch on here to work. What tool are you using to create it? TortoiseSVN certainly didn't like it. I initially got the error, "The chunk size did not match the number of added/removed lines!".
sorry, I removed a dubbled line with an editor. Just my fault! :oops:
What does that mean - the circumstances? I determine what circumstances prevail. -- Napoleon Bonaparte
---
If we cannot end now our differences, at least we can help make the world safe for diversity. -- John F. Kennedy
---
Our problems are man-made, therefore they may be solved by man. No problem of human destiny is beyond human beings. -- John F. Kennedy
Bjarni
Tycoon
Tycoon
Posts: 2088
Joined: 08 Mar 2004 13:10

Post by Bjarni »

skidd13 wrote:sorry, I removed a dubbled line with an editor. Just my fault! :oops:
Never do that. Each time I tried to do that, I broke the diff, so after a few times, I completely stopped doing it. Using svn to make the diff will ensure that it will live up to the standard so it can be merged :wink:
User avatar
jez
Traffic Manager
Traffic Manager
Posts: 158
Joined: 23 Aug 2003 21:24

Post by jez »

skidd13: Why did you hyphenate Gesichts-Nummerncode?
=== Jez ===
Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 42 guests