Maybe it allows to set the dir hirarchy or 'patch level' (I know it's wrong word, what's the correct one) to which to apply the patch. This patch applies with -p1 (default for hg / git) instead of -p0 (svn).robotboy wrote:The patch suplied seems to look for english.txt under src/a/lang/english.txt which doesn't exist.
Edit: I am trying to patch on Windows 7 with Tortoise Merge.
[Patch] Improved Build Station GUI [r19159]
Moderator: OpenTTD Developers
- planetmaker
- OpenTTD Developer
- Posts: 9432
- Joined: 07 Nov 2007 22:44
- Location: Sol d
Re: [Patch] Improved Build Station GUI [r19159]
OpenTTD: manual | online content | translations | Wanted contributions and patches
#openttdcoop: blog | wiki | public server | DevZone | NewGRF web translator
DevZone - home of the free NewGRFs: OpenSFX | OpenMSX | OpenGFX | Swedish Rails | OpenGFX+ Trains|RV|Industries|Airports|Landscape | NML
Re: [Patch] Improved Build Station GUI [r19159]
I don't understand why people use patch files which don't include any kind of source control changelist ID. They're so much harder to apply than one that specifies what version of the file(s) it was created from.
Melt with the Shadows,
Embrace your destiny...
Embrace your destiny...
Re: [Patch] Improved Build Station GUI [r19159]
I believe its a mercurial patch, so it has the changeset info just not for subversion.
Re: [Patch] Improved Build Station GUI [r19159]
So it does, I missed the id first time
Melt with the Shadows,
Embrace your destiny...
Embrace your destiny...
Re: [Patch] Improved Build Station GUI [r19159]
Maybe it helps if you add the prefix level and the tool you used to the filename, something like we did for is2: http://bundles.openttdcoop.org/is2/trunk-patches/
Town Names: Portuguese Belarusian French Swiss · Temperate Lumber Mill
Still work in progress: OpenGFX or/and OpenSFX - Please help!
-
- Tycoon
- Posts: 1828
- Joined: 10 Jul 2006 00:43
- Location: Spain
Re: [Patch] Improved Build Station GUI [r19159]
Thanks for update this patch. Tooltips should be used also in the window with way-points.
Sorry if my english is too poor, I want learn it, but it isn't too easy.
- [list][*]Why use PNG screenshots in 8 bpp games.
[*]Caravan site New Industry. · Spain set. · Some spanish trains for locomotion[*]Favourites:GRVTS · ECS · FIRS
Re: [Patch] Improved Build Station GUI [r19159]
I finaly got the patch to apply but MSVC Pro will not compile it. The errors I get are:
Error 1 error C2059: syntax error : '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1033 openttd
Error 2 error C2143: syntax error : missing ';' before '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1033 openttd
Error 3 error C2143: syntax error : missing ';' before '}' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1033 openttd
Error 4 error C2059: syntax error : '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1072 openttd
Error 5 error C2143: syntax error : missing ';' before '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1072 openttd
Error 6 error C2143: syntax error : missing ';' before '}' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1072 openttd
Error 7 error C2059: syntax error : '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1146 openttd
Error 8 error C2143: syntax error : missing ';' before '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1146 openttd
Error 9 error C2143: syntax error : missing ';' before '}' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1146 openttd
Error 1 error C2059: syntax error : '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1033 openttd
Error 2 error C2143: syntax error : missing ';' before '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1033 openttd
Error 3 error C2143: syntax error : missing ';' before '}' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1033 openttd
Error 4 error C2059: syntax error : '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1072 openttd
Error 5 error C2143: syntax error : missing ';' before '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1072 openttd
Error 6 error C2143: syntax error : missing ';' before '}' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1072 openttd
Error 7 error C2059: syntax error : '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1146 openttd
Error 8 error C2143: syntax error : missing ';' before '{' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1146 openttd
Error 9 error C2143: syntax error : missing ';' before '}' c:\Users\Leo.Giusti\Documents\Visual Studio 2008\Projects\OpenTTD\r19145\src\rail_gui.cpp 1146 openttd
Formerly known as r0b0t_b0y2003, robotboy, roboboy and beclawat. The best place to get the most recent nightly builds of TTDPatch is: http://roboboy.users.tt-forums.net/TTDPatch/nightlies/
Re: [Patch] Improved Build Station GUI [v5.1-r19159]
I learned the basics of nested widgets while working on this patch.scoz wrote:[...] not having the time to learn the nested widgets system [...]
Thank you. Glad to hear that the work done on this patch pleased you.scoz wrote:I'll update the first post [...]
I use mercurial and should have mentioned it. I've done a subversion checkout and will provide patch files for '-p1' and svn users.Ammler wrote:Maybe it helps if you add the prefix level and the tool you used to the filename, something like we did for is2: [url]http://bundles.openttdcoop.org/is2/trunk-patches/[url]
Seems like MSVC don't like multi-field assignments to struct variables, something that my gcc (4.4.3-20100108) doesn't complain about. I think it's fixed now but can't/will not test it on Windows.robotboy wrote:I finaly got the patch to apply but MSVC Pro will not compile it. [...]
I've found and fixed another bug: with an height-increased window, the station coverage area resize down also reduced size of the whole window.
The version number is bumped to 5.1 (considering that v3-sbr1 is v5 as scoz previously released a v4).
- Attachments
-
- station-build-gui_v5.1-r19159.patch
- for subversion users
- (29.82 KiB) Downloaded 148 times
-
- station-build-gui_v5.1-hg-ea541d92bafc.patch
- against r19159, for '-p1' users
- (29.62 KiB) Downloaded 124 times
Re: [Patch] Improved Build Station GUI [r19159]
He nice work.
But we generate the patches automatically from the repo, that is why we provide both versions. If you make them manually, one version should be fine, just mention it in the filename, so it is obviously how to patch (-p0 or -p1, a svn version is nice for the Tortoise users, as those can't patch with hg/git diffs)
--> so if you make a svn patch, that is fine for all, imo
Greets
Ammler
But we generate the patches automatically from the repo, that is why we provide both versions. If you make them manually, one version should be fine, just mention it in the filename, so it is obviously how to patch (-p0 or -p1, a svn version is nice for the Tortoise users, as those can't patch with hg/git diffs)
--> so if you make a svn patch, that is fine for all, imo
Greets
Ammler
Town Names: Portuguese Belarusian French Swiss · Temperate Lumber Mill
Still work in progress: OpenGFX or/and OpenSFX - Please help!
Re: [Patch] Improved Build Station GUI [r19159]
MSVC is happy now and builds it fine.
Formerly known as r0b0t_b0y2003, robotboy, roboboy and beclawat. The best place to get the most recent nightly builds of TTDPatch is: http://roboboy.users.tt-forums.net/TTDPatch/nightlies/
Re: [Patch] Improved Build Station GUI [r19389]
Please find an updated version of this patch (r19389):
- Unified rail station widget descriptions (r19321)
- No more parameter setting: the current newstations gui is totally replaced
- Fix: Ensure that the last selected station's class/type are visible over gui invocation
- Attachments
-
- station-build-gui_v6.0-r19389.patch
- for subversion users
- (38.85 KiB) Downloaded 245 times
-
- station-build-gui_v6.0-hg-671246bf9a84.patch
- against r19389, for '-p1' users
- (38.77 KiB) Downloaded 147 times
Re: [Patch] Improved Build Station GUI [r19396]
A new update against r19396:
- Don't introduce a new widget for 'station coverage area' texts.
- Simplify code for restoring window size: don't depend of previous window size but on previous station types matrix rows/columns count. That allow to restore window size if 'line height' change between two gui invocation.
- Break up patch into smaller parts.
- Attachments
-
- station-build-gui_v6.1-r19396.patch
- for subversion users
- (36.72 KiB) Downloaded 208 times
-
- station-build-gui_v6.1-hg-b753aec66152.patch
- against r19396, for '-p1' users
- (36.73 KiB) Downloaded 140 times
-
- station-build-gui_v6.1-hg-b753aec66152_small-patches.tar
- against r19396, small patches, for '-p1' users
- (60 KiB) Downloaded 149 times
Re: [Patch] Improved Build Station GUI [r19159]
I have been checking the patch list. I don't know much about station NewGRFs or the complicated parts of the GUI, so I might be wrong at some details.
Before 1), the new station class list was checked each time it was required. After 1), the list is generated once and used until the window is closed. Generating the list once is more efficient, but maybe it was done every time because something can make this list change over time. If this is not the case, I think the new station should be generated only once too.
This comment in 2) could go directly in 1):
After 5), I think that the window is complex enough to be made pinnable.
Before 1), the new station class list was checked each time it was required. After 1), the list is generated once and used until the window is closed. Generating the list once is more efficient, but maybe it was done every time because something can make this list change over time. If this is not the case, I think the new station should be generated only once too.
This comment in 2) could go directly in 1):
Code: Select all
+ uint line_height; ///< Height of a single line in the newstation classes and types selection matrices
+ /// (#BRSW_NEWST_STATION_CLASSES_LIST & #BRSW_NEWST_STATION_TYPES_LIST widgets).
Spanish translation of OpenTTD
Extended heightmaps
Have fun, don't quarrel too much and add as many advanced settings as you can.
Extended heightmaps
Have fun, don't quarrel too much and add as many advanced settings as you can.
Re: [Patch] Improved Build Station GUI [r19159]
Maybe it changes after changing newgrfs ingame (yes that option with big, red, warning box).
Correct me If I am wrong - PM me if my English is bad
AIAI - AI for OpenTTD
AIAI - AI for OpenTTD
Re: [Patch] Improved Build Station GUI [r19396]
Thanks for having took the time to check it; to you and others who have/will do it. I'm (slowly) learning about the OpenTTD codebase while developing patches and should certainly miss a lot of its subtleties.Terkhen wrote:I have been checking the patch list. I don't know much about station NewGRFs or the complicated parts of the GUI, so I might be wrong at some details.
The previous versions made by scoz already cached the station class list. When updating the code I should admit that I've not investigate implications of that choice.Terkhen wrote:Before 1), the new station class list was checked each time it was required. After 1), the list is generated once and used until the window is closed. Generating the list once is more efficient, but maybe it was done every time because something can make this list change over time. If this is not the case, I think the new station should be generated only once too.Kogut wrote:Maybe it changes after changing newgrfs ingame (yes that option with big, red, warning box).
I've looked deeper in the NewGRF loading code after reading your comments. As I understand it, station classes are stored in 'newgrf_station.cpp:_station_classes'. That array is fill when generating world, loading game/scenario or changing the NewGRF list.
In the first two cases, the BuildRailStation window can't exists. In the latter case, the whole screen is marked as dirty. If the BuildRailStation window is loaded, it will be repaint but not reinitialized. The cached station classes of the patched window isn't updated and the window is repainted with invalid data. In contrary, the actual window (and since its introduction in r4768) will correctly reflect all in-game's NewGRF changes.
The default behavior should be preserved either by refreshing the station classes cache on NewGRF changes or by not caching them anymore. I'll investigate these two options.
Between '01' and '02' this comment differs by two things:Terkhen wrote:This comment in 2) could go directly in 1):
Code: Select all
+ uint line_height; ///< Height of a single line in the newstation classes and types selection matrices + /// (#BRSW_NEWST_STATION_CLASSES_LIST & #BRSW_NEWST_STATION_TYPES_LIST widgets).
- one line comment in '01', two line in '02'
- '#BRSW_NEWST_LIST' in '01', '#BRSW_NEWST_STATION_TYPES_LIST' in '02'
Why not, that could be useful to draw eye-candy tiles while building, for example, a road complex. I need to examine the interactions with the window when the selected rail type/transportation type change.Terkhen wrote:After 5), I think that the window is complex enough to be made pinnable.
Re: [Patch] Improved Build Station GUI [r19396]
About the NewGRFs, I was referring to callbacks. I don't know if rail stations use callbacks, though.
I meant pinning only the build rail station window, but I like this idea. But since it is complex and not directly related to this feature, you should implement it separately.sbr wrote:Why not, that could be useful to draw eye-candy tiles while building, for example, a road complex. I need to examine the interactions with the window when the selected rail type/transportation type change.
Spanish translation of OpenTTD
Extended heightmaps
Have fun, don't quarrel too much and add as many advanced settings as you can.
Extended heightmaps
Have fun, don't quarrel too much and add as many advanced settings as you can.
Re: [Patch] Improved Build Station GUI [r19396]
I've done a little bit more research about newstations in NewGRF and the OpenTTD code that deal with it. However, to anyone who's more skilled on this subject, feel free to correct me if I'm making wrong assertions.
From the NewGRF spec:
All these callbacks refers to station types. Only CB 13 is of interest for the BuildRailStationWindow. CB 13 is checked, in both the trunk and patched version, inside DrawWidget() and OnClick().
How OpenTTD handle station classes:
The station class list is build inside OnInit() and is rebuild each time the window is reinitialized.
There is three case where BuildRailStationWindow::OnInit() is called:
However station class names are obtained by GetStationClassName() that do an array look up. I don't think it's worth caching them and am tempted to reduce _railstation.station_classes to a simple mapping between matrix position and station class ID.
On a side note, we can use OnInit() to ensure that _railstation.station_class is always valid (both in trunk and in patch):
About pinning the window:
I'll update patches and post an updated version soon.
From the NewGRF spec:
- The station class ID is defined by an Action 0 / Feature 04 / Property 08.
- The station class name is defined by one Action 4 / Feature 04 per language.
- There can be no more than 16 station class defined.
In newgrf_callbacks.h I've found the following callbacks referable to stations:Terkhen wrote:I don't know if rail stations use callbacks, though.
Code: Select all
41: CBID_STATION_AVAILABILITY = 0x13, // 8 bit callback
44: CBID_STATION_SPRITE_LAYOUT = 0x14,
94: CBID_STATION_TILE_LAYOUT = 0x24, // 15 bit callback
177: CBID_STATION_ANIM_START_STOP = 0x140, // 15 bit callback
180: CBID_STATION_ANIM_NEXT_FRAME = 0x141, // 15 bit callback
183: CBID_STATION_ANIMATION_SPEED = 0x142, // 8 bit callback
204: CBID_STATION_LAND_SLOPE_CHECK = 0x149, // 15 bit callback, not implemented
How OpenTTD handle station classes:
- The station classes are stored in 'static StationClass _station_classes[STAT_CLASS_MAX]'.
- This array is indexed by StationClassID vars.
- There can be no more than 32 (STAT_CLASS_MAX) station class defined.
- The station class ID defined in the NewGRF is stored in StationClass.id.
- The station class names are mapped to a StringID in StationClass.name.
- Values of the station class array can be accessed by functions defined in newgrf_station.h.
- The station class array can be altered by ResetStationClasses() and AllocateStationClass(...) that are only called when the NewGRFs are (re)loaded.
The station class list is build inside OnInit() and is rebuild each time the window is reinitialized.
There is three case where BuildRailStationWindow::OnInit() is called:
- when building the window: Window::FinishInitNested(...) → Window::InitializeData(...) → Window::OnInit().
- when changing the used language: GameOptionsWindow::OnDropdownSelect(GOW_LANG_DROPDOWN, ...) → ReInitAllWindows() → Window::ReInit(...) → Window::OnInit().
- when updating the NewGRF list: NewGRFWindow::OnClick(..., SNGRFS_APPLY_CHANGES, ...) → NewGRFConfirmationCallback(...) → ReloadNewGRFData() → LoadStringWidthTable() → ReInitAllWindows() → Window::ReInit(...) → Window::OnInit().
However station class names are obtained by GetStationClassName() that do an array look up. I don't think it's worth caching them and am tempted to reduce _railstation.station_classes to a simple mapping between matrix position and station class ID.
On a side note, we can use OnInit() to ensure that _railstation.station_class is always valid (both in trunk and in patch):
Code: Select all
virtual void OnInit()
{
if (_railstation.newstations && _railstation.station_class != STAT_CLASS_DFLT) {
uint num_station_classes = GetNumStationClasses();
if (_railstation.station_class >= num_station_classes) {
_railstation.station_class = (StationClassID) (num_station_classes - 1);
_railstation.station_type = 0;
}
}
}
About pinning the window:
and previouslyTerkhen wrote:I meant pinning only the build rail station window, but I like this idea. But since it is complex and not directly related to this feature, you should implement it separately.sbr wrote:Why not, that could be useful to draw eye-candy tiles while building, for example, a road complex. I need to examine the interactions with the window when the selected rail type/transportation type change.
For me, inside a running game, pinned windows can only be closed by clicking on the close box or typing Ctrl+Del. Perhaps you were thinking of pinning the BuildRailStationWindow in the context of the rail toolbar?Terkhen wrote:After 5), I think that the window is complex enough to be made pinnable.
I'll update patches and post an updated version soon.
Re: [Patch] Improved Build Station GUI [r19159]
You would be best doing the caching inside OnInvalidateData, and then calling InvalidateWindowData or InvalidateWindowClassData to invdlidate it whenever the NewGRF list is updated
Re: [Patch] Improved Build Station GUI [r19396]
I agree: it is not worth to cache the name too. Since it is a small vector with a known maximum size (STAT_CLASS_MAX) I think that a simple array and an int size should be enough to store it.sbr wrote:To summarize, OnInit() is, in my opinion, the right place to cache the station class IDs and names list as it'll be reloaded when the cached values changes (in contrary to what I previously wrote).
However station class names are obtained by GetStationClassName() that do an array look up. I don't think it's worth caching them and am tempted to reduce _railstation.station_classes to a simple mapping between matrix position and station class ID.
If I understood it correctly, OnInit always caches the station class array. Since OnInit is called after updating the NewGRF list, shouldn't it always be valid after that?sbr wrote:On a side note, we can use OnInit() to ensure that _railstation.station_class is always valid (both in trunk and in patch):
With pinnable I meant the same as you do, but right now if you change toolbars the build rail station window is always closed (probably even if made pinnable). A different behaviour is not trivial, since it would require to think and implement when it should be closed and when it shouldn't. Because of that, I think it is better to left it out for now.sbr wrote:For me, inside a running game, pinned windows can only be closed by clicking on the close box or typing Ctrl+Del. Perhaps you were thinking of pinning the BuildRailStationWindow in the context of the rail toolbar?.
Spanish translation of OpenTTD
Extended heightmaps
Have fun, don't quarrel too much and add as many advanced settings as you can.
Extended heightmaps
Have fun, don't quarrel too much and add as many advanced settings as you can.
Re: [Patch] Improved Build Station GUI [r19525]
I've replaced the caching by a mapping between station class id and matrix position in a StationClassIDByte array.Terkhen wrote:I agree: it is not worth to cache the name too. Since it is a small vector with a known maximum size (STAT_CLASS_MAX) I think that a simple array and an int size should be enough to store it.
The mapping is now done in OnInvalidateData(...) that is explicitly triggered in ReloadNewGRFData(). OnInit() had the advantage to work out-of-the-box but, as you pointed, OnInvalidateData(...) is the right method to use to reflect external changes that impact the window.Owen wrote:You would be best doing the caching inside OnInvalidateData, and then calling InvalidateWindowData or InvalidateWindowClassData to invdlidate it whenever the NewGRF list is updated
'_railstation.station_class' is the currently selected station class. When removing a NewGRF, it can point to a station class that doesn't exists anymore. I now check and update if needed the window settings (_railstation.station_{class, type, count}).Terkhen wrote:If I understood it correctly, OnInit always caches the station class array. Since OnInit is called after updating the NewGRF list, shouldn't it always be valid after that?sbr wrote:On a side note, we can use OnInit() to ensure that _railstation.station_class is always valid (both in trunk and in patch):
I think too.Terkhen wrote:With pinnable I meant the same as you do, but right now if you change toolbars the build rail station window is always closed (probably even if made pinnable). A different behaviour is not trivial, since it would require to think and implement when it should be closed and when it shouldn't. Because of that, I think it is better to left it out for now.
Attached is an updated version against r19525:
- Support NewGRF changes while the window is open:
- always point to valid station class and type, update scrollbars
- switch between default and newstations window if needed
- Rework small patches numbers:
- patches 0* applies to the BuildRailStationWindow while keeping the same appearance as in trunk
- patches 1* defines the new look of the window (station classes in matrix, preview of station types in right matrix)
- patches 2* enhance the new window (tooltip, resizable, restore size)
- Attachments
-
- station-build-gui_v6.2-r19525.patch
- for subversion users
- (39.56 KiB) Downloaded 184 times
-
- station-build-gui_v6.2-hg-e7ca2b0ec896.patch
- against r19525, for '-p1' users
- (39.53 KiB) Downloaded 114 times
-
- station-build-gui_v6.2-hg-e7ca2b0ec896_small-patches.tar
- against r19525, small patches, for '-p1' users
- (80 KiB) Downloaded 185 times
Who is online
Users browsing this forum: Majestic-12 [Bot] and 55 guests