3298's patches

[3298]

There is one thing about OpenTTD that recently changed:
As you may have seen, OpenTTD recently switched to GitHub. This affects the primary code repository as well as the bugtracker. All bugs and patches are to be reported there now, FlySpray is in read-only mode since late Friday evening, the openttd.org-hosted repositories followed suit on Saturday.
However, there's also something that didn't change:
I do not agree with GitHub's Terms of Service and Privacy Policy, so I am not going to use that bugtracker.

I may not be the most active contributor (not even close), but I did write some patches, most of which are still not merged (and some may never be). However, the combination of the facts listed above deprives me of a way to contribute, so I'm releasing what is most likely going to be the final version of my patches.
Since the last previously released version of some of my patches might not apply cleanly anymore, I'm using this post to dump them all so others have a chance to use them, include them in patchpacks, or even convince the devs to merge them into trunk. All of them are updated to the last SVN revision in existence, r28004.
I probably shouldn't even need to, but because there always are some people who don't get the notice: Despite the similarity between r28004 and release version 1.8.0, applying patches to a release is a BAAAD idea. The purpose of a release is that the code is guaranteed to be a certain version in order to track down potential bugs. Patching changes the code and therefore defeats this purpose.
With that out of the way, here are the patches. I consider all of them to be fully client-side, that is, they should work in multiplayer even if only some players use them. That doesn't mean it's a good idea to actually mix and match modified and pristine versions, though - there might be some corner case I missed. Either way, use them at your own risk.

1. Enhanced Transparency Window
Based on a discussion in 2011, this was my 'debut' in terms of patching, so to speak, when I posted it in 2013. After about a month of discussing it, the thread went silent.
About the patch itself: it revamps the transparency window to include all options related to transparency or visibility of stuff on the map. On top of that, I designed it to be more versatile than the original one by putting two rows of buttons with three states each (unless the button's option can only be visible or not, then it's obviously only two states) into the window. This allows configuring the main transparency hotkey (default: X) to toggle an option between transparent and invisible, for instance (might be useful for signs or trees), because this hotkey now toggles which of the two rows is active. Locking is gone, just configure the two rows for an option to the same state (locking was pretty hidden anyway, needing a Ctrl-click). The other transparency hotkeys are largely unchanged; I merely adjusted them for the expanded and re-sorted selection of options (for which I needed another row of keys: instead of 1-9, it's 1-9, 0, F-L now), adjusted the Ctrl+hotkey and Ctrl+Shift+Hotkey so they perform the action closest to their old meaning (Ctrl-hotkey: swap the two rows for a single option (also doable with Ctrl-click onto the option), Ctrl+Shift-hotkey: set single option in active row to invisible or opaque, depending on current state), and added Shift+hotkey (set single option in active row to transparent or opaque, depending on current state).
The patch is divided into four parts:
- Part 1 gives the sign list its own copy of the "show competitor signs" option instead of depending on what's shown on the map.
- Part 2 moves all options related to transparency and invisibility to the transparency window and re-sorts them. "Full animation" becomes a console/config-only option like the blitter.
- Part 3 adds a resize button to the transparency window because it got pretty large with all the added options. The small version merges all buildings (houses, industries, stations) into one option, hides the fences&lamps option (the former "Full detail"), and merges all signs (normal signs, station signs, waypoint signs), and hides the competitor signs option.
- Part 4 finally adds the second row of settings.
You can stop at any point in this series if you want only some features.
Caveats with this patch: all old transparency/invisibility settings are ignored. OpenTTD has a system for converting old settings, but as far as I can tell, that only works for savegame-stored settings (by converting them upon loading an old savegame). These are global settings however, and the config doesn't know what version it was saved with. Also, back when I first released it, a dev pointed out that double-tapping the main transparency hotkey does not revert everything but the locked options to opaque anymore, but I consider that to be working as intended.

2. Faster order creation
Written as an alternative to adf88's patch (his successor patch is here) which allows creating orders with certain modifiers when Shift-clicking or Ctrl-Shift-clicking on a location (in addition to the existing Ctrl-click feature), this patch intends to help with quickly setting up orders with modifiers. As such, I ended up posting it into his thread (the older one, because the new one didn't exist yet). I didn't write this entirely on my own; Eagle_rainbow helped me, especially in the ideas and testing department.
Instead of adding Shift-click and Ctrl-Shift-click actions, I opted for a radically different approach: with the patch, you can select order modifiers with the normal modifier buttons and their hotkeys before selecting the order destination. Ctrl-click continues to exist for compatibility reasons, and it overrides the selected modifiers if you try to combine the approaches.
As an example, this is how you could set up an order to unload normally and take any other cargo with auto-refit:
- Click "Go To" if this mode isn't already active (the old "quick go-to" has been helping with that for years by activating the mode upon opening an order window with less than two orders)
- Click the little arrow next to "Refit" to open the refit dropdown
- Choose "Refit to available" from the dropdown
- Click on the destination station
I had to move some of the dropdowns around to make sure all of them are accessible - before selecting a destination, OpenTTD cannot know if an order is going to be a depot order or not, so the service dropdown and the load/unload dropdowns have to be visible when selecting the modifiers for an order. To keep the order window at a somewhat sane width, that implied moving the non-stop dropdown to the other row.
I intended this to go into trunk, so all of the functionality, including this alternate layout, is hidden behind a setting that's off by default. That was the best way I could ensure that no players would be confused by this change; they would continue to use the window they were used to, but now with the option to use a better one. The only changes not covered by the setting are some additional hotkeys: Ctrl-H for go_via (H was non-stop already, which is in the same dropdown, so this just felt natural), Ctrl-G as a new default for nearest_depot from the go-to dropdown (the hotkey existed before, but wasn't associated to a real key unless hotkeys.cfg was modified; that also causes this modification to not go into effect unless manually applying it or deleting hotkeys.cfg to let it regenerate), Ctrl-Shift-G for conditional order creation (this one didn't exist before), Ctrl-Y as a new default for always_service from the service dropdown (existed before; QWERTZ keyboard users like myself may want to rebind it to Ctrl-Z to compensate for the swapped keys; in the previous release of the patch I actually specified Ctrl-Z as the default), Ctrl-U as new default for transfer, Ctrl-J as new default for no_load, Ctrl-K as new default for no_unload. Note that some of these new hotkeys conflict with new hotkeys in the transparency window patch above, so when combining them I'd advise inventing new defaults.
The patch is split into no less than 9 parts, though stopping halfway through the series makes little sense. The splitting has been done solely to aid in understanding the code changes.
1. Preparation: Allow the refit window to return the selection instead of issuing a DoCommand
2. Preparation: Split some code that applies an OrderDepotAction to an order into a separate function
3. Preparation: Move GetOrderCmdFromTile into struct OrdersWindow for access to certain variables in later parts
4. Preparation: Introduce member variables in struct OrdersWindow that will hold the order modifiers for an order that's under construction
5. Add the new order window layout
6. Show the texts of the current order modifiers on the buttons at all times, because when an order isn't fully created yet there is no line in the order list to show them
7. Actually allow selecting order modifiers for an order under construction when the new layout is in use
8. Add/Change the hotkeys
9. A little optimization to quick_goto so it reverts to go-to after inserting a conditional or nearest-depot order in the new layout instead of disabling go-to mode or doing nothing, respectively.

Special thanks to Eagle_rainbow, who not only provided tons of ideas, but also tested this patch, updated it once, and wrote a few lines of code which might even be in there still in spite of later changes and rebases. You can read the discussion in adf88's first thread to get an idea of how we got from a collection of ideas to a full patch.

3. Group window changes
This is a collection of changes from FlySpray, some of them by myself, some of them as response to feature requests by others. The common theme is that they deal with groups.
1. The first one consists of three little fixes where the group and autoreplace windows weren't properly updating each other. The issues were introduced with the group hierarchy.
2. The next one is trivial (FS#6491): Instead of creating a group, then dragging it onto another to make the new one a subgroup of the old one (which could get annoying when there are enough groups for the group window's scrollbar to become relevant, and the old group and new group end up sorted far from each other), how about selecting the old group and then creating the new one directly as a subgroup? That's what I had in mind when I wrote and submitted the second change. Creating a top-level group is possible by making sure no group is selected (okay, when the scrollbar becomes relevant, this is kind of hard to do, but read on), or when the "All vehicles" or by selecting "Ungrouped vehicles" beforehand (those are always at the top of the list).
3. The third change is similarly trivial (FS#5977): Someone wanted a proper error message when changing the group hierarchy failed to prevent a cycle. Not much to say about it, the patch adds a reason text to the error message. On some occasions posting a patch was enough to accelerate the process of getting a requested feature into trunk, but not this time.
4. The fourth change is about the info displayed for a group. The vehicle count and the profit icon for a group were not updated to take subgroups into account when the group hierarchy was introduced. Someone made a feature request about it (FS#6189), and I independently discovered it while working on the next item in this list. Note that the feature request has an interesting idea which I didn't use when initially writing my version: showing both the group's own vehicle count and the group's accumulated vehicle count including subgroups. I responded with a slight refinement of the display format from "own_vehicles/own_and_subgroup_vehicles" to "own_vehicles (+subgroup_vehicles)", but the resulting code hasn't been released until now.
If you check the bugtracker, you'll find a portion of the discussion, including all the older versions of the code, over at the ticket for the next change. Also, that ticket contains two variants (not counting the earlier versions with issues) which differ only in code. The current code is based on the second variant.
5. The last one is another useful little feature: collapsible groups. Also a feature request I implemented (FS#6053). There's not much to write about it, but it's most likely a welcome addition for those who use hierarchical groups a lot.
These changes are largely independent from each other, though with the last one applied having the fourth one will become more important. If change number 2 is not used, then there will be a small error when applying the last one because they both add lines to a certain function (the constructor of struct Group). That's enough for most patch-applying tools to bail out, but with any experience with applying patches manually this should be easy to fix.

4. Implicit order toggle button
Back to orders - there is another feature that's been requested a few times already. On the bugtracker this is known as FS#4540, and there is a forum thread as well. Both contain patches by others, I've merely written yet another competing implementation (while taking inspiration from the others). My picks were leaving the implicit order generation alone (fonso disabled it entirely), putting the toggle button into the window (like billw, but in a different position because I don't think the title bar is the proper place for such a button), and leaving the order numbering scheme as it is (which provides another hint that there are hidden orders). While I was there, I added a few hotkeys (not bound to any key by default, but now they can be configured in hotkeys.cfg). Unlike the older patches, the timetable window has been changed too - it works, but the code isn't pretty, mostly due to an ancient design decision dating back to the introduction of timetables.
Due to the added button in the orders window and the added hotkeys, this clashes with the faster order creation patch above. I've created a special version of the implicit order patch for applying on top of the faster orders one. The orders window gets pretty wide with five buttons next to each other, but oh well - that's the price to pay for more and more and even more functions.

5. Window placement
One common complaint about the orders and timetable windows is how they obscure most of the vehicle's main window when opened. I wrote a patch that not only fixes that, but also improves window placement in general.
To explain what it does, I'll need to outline how OpenTTD places its windows without the patch:
1. In the top-left corner.
2. Going through all open from top-most to bottom-most, next to an existing window with one corner of the new window on top of the corresponding corner of the other window. All edges and corners are considered.
3. Same as the previous step, but also allowing placement partly off the screen. For some reason, only one of the two corners delimiting the edge of the existing window is considered.
4. Along a diagonal starting in the top-left corner at fixed intervals.
The first valid position found is where the window will spawn. In the first three steps, a position is discarded if it covers any existing windows. In the last step, a position is discarded if this causes the top-left corner of a window to end up on top of the top-left corner of another window.
So what's wrong with this? First off, the fourth step will eventually place windows entirely outside the screen, making them completely unreachable. You need to use Del or Shift-Del to get rid of them (along with many other windows), then re-open them. The third step has a pointless (probably historical, i.e. inherited from TTD) limitation; the second and third step consider tooltips to be normal windows (probably an oversight). Finally, covering other windows is treated as strictly forbidden, even when it's only about two or three pixels along the edge, until the time for the fourth step comes - then it's suddenly fine to stack several windows on top of each other with slight offset.
My solution is fairly radical. The most straight-forward part is doing away with the strange limitation of step 3 while merging it with step 2 and making it ignore any tooltips. Now it gets more complicated: I made the window placement code rate a spot, adding up certain penalties for obscuring other windows or being partially off-screen. The spot with the lowest penalty is where the window ends up. However, since there are some windows with inherently higher priority (some settings windows, errors, etc.), that might cause a new window to be hidden underneath an existing one. Therefore the code now checks for priority and discards spots colliding with higher-priority windows (except tooltips, as I already mentioned) entirely. If that means there are no valid positions left, the fallback position is top-left instead of the old step 4. I consider that to be rare enough to emit a warning to the terminal.
Now let's go back to the orders and timetable windows. The problem wasn't limited to them actually - it affected all child windows of other windows that didn't override their position like most settings windows. (Toolbar windows get special treatment too, and that one is actually in the general window placement code instead of the windows themselves, but I didn't change it as it's working well.) For instance, the vehicle info window (reached from the bottommost button in the main vehicle window) got the same treatment as the orders and timetable ones. They were placed at a fixed offset compared to the parent window which looked an awful lot like the offset used in step 4. So how did I change it, and how is this connected to the placement penalties? - Simple, when a window with a parent window gets placed in this patch, all spots which are not next to the parent get a penalty. That way a nearby spot is favored so you don't have to search for the new window all over your huge 4K screen, without covering the parent window; but if the area around the parent window is really unfavorable, the new child can go somewhere else too.
The penalties are defined in the source code and are not meant to be configured by users.
The patch comes in two parts again; the first adds the penalty system, the second takes care of the child window placement.


So there you have it, an inventory of the noteworthy portion of my patches, ready to be used.
That leaves just one more thing for me to say: Goodbye, everyone.

[andythenorth]

Thanks for the patches 3298.

I'm sorry that the move to GitHub won't work for you. It's intended to make OpenTTD development sustainable in the long run. Thank you for your previous contributions.

[orudge]

I do not agree with GitHub's Terms of Service and Privacy Policy, so I am not going to use that bugtracker.

Out of interest, what in there don't you agree with?

[HackaLittleBit]

Out of interest, what in there don't you agree with?

One of them is here.

We may share User Personal Information if we are involved in a merger, sale, or acquisition. If any such change of ownership happens, we will ensure that it is under terms that preserve the confidentiality of User Personal Information, and we will notify you on our website or by email before any transfer of your User Personal Information. The organization receiving any User Personal Information will have to honor any promises we have made in our Privacy Statement or in our Terms of Service.

[3298]

ic111 asked me the same question in a PM (among some other questions), prompted by the curious timing of our respective topics.
Here's what I answered:

I find the privacy measures insufficient. I want my profile to return "403 Forbidden" or something equivalent to everyone. I don't really mind having public repositories (private ones cost $7 per month, which is $7 more than I'm willing to pay for stuff I do for fun), as long as my profile page with mail address, activity, list of repos (making them hard to find unless you know the URL already), and everything else on it is off-limits for everyone else. That's impossible to get, no matter how much you pay. And trying that half-a-workaround of using a swarm of accounts is explicitly forbidden.
I'm simply allergic to any form of tracking. End of the story.

His reply included this line, which summarizes it pretty well:

Maybe, in the end, that's another instance of "you pay with your data".

... which I find unacceptable. I'm not using so-called social networks for exactly that reason. As the worst offender, Facebook of course gets the most flak - I made all FB domains resolve to 127.0.0.1 for me, just to prevent accidental data leaks.
Now you have an idea of how strong I feel about privacy, (re-?)read the ToS and PP and evaluate them from that viewpoint. GitHub is somewhat tame compared to FB & such, but you'll find that there is not much info about exactly what parts of a profile are public - the list of public repositories, for example (I had to look at other people's profiles and some StackExchange questions to find that out). As I wrote in the PM, I'm fine with the repos themselves being public, but I disagree with this list as it allows others to get answers to the question "what else is this person working on?" with no effort, which clearly fits the definition of tracking.
If this is considered to be an extreme view instead of normal, the world is in a sad state.

By the way, the advertising and analytics notification appearing on this forum is almost as distressing.

[orudge]

By the way, the advertising and analytics notification appearing on this forum is almost as distressing.

This is a bit off-topic for here, but that is an EU requirement based upon the fact we have Google adverts on the forum, which do indeed track you. In theory you can disable this (although of course Google will need to set a cookie to do this). We otherwise do not "track" you in any way.

[andythenorth]

If this is considered to be an extreme view instead of normal, the world is in a sad state.

I think it's at the more extreme end. Current news events might shift the spectrum though.

I had a related problem, in that I need to keep my work (paid job) disconnected from OpenTTD. My github profile was linked to work projects, and there was no good way to resolve that. Ultimately we have private repos elsewhere for work so I disconnected my profile from work github projects and open source contributions.

Github ToS allow only one account per real person as far as I can understand. The idea seems to be to show everything about a contributor.

It's a form of transparency that some find benign and helpful, and others find intrusive and untenable.

Life is too short to be hosting repos and issue trackers ourselves. So yair, I am prepared to be data-mined within strict rules, if that's the cost of entry.

I don't like that the trade-off has to be made, but I'll accept it so we can go the github route. I can understand others may not.

[HackaLittleBit]

Just do a search in google.
Scroll to the bottom of the page and you can see that they know were you live.
I never gave that info to them.
Very scary!!!!!!!!!!

Edit:
P.S. And I don't live in the place of the IP address, that my ISP gives.

[JGR]

I for one have separate work and personal github accounts, registered to and committed to using work and personal email addresses respectively.
As long as you are non-abusive, work/individual identities is not likely to be a problem. Much of what is in ToSs is not enforceable or realistic anyway.

[Kruemelchen]

Just do a search in google.
Scroll to the bottom of the page and you can see that they know were you live.
I never gave that info to them.
Very scary!!!!!!!!!!

A simple explanation on that, google of course knows your IP address like every website you visit for obvious technical reasons. That said, any website can "track" your IP address down to the spot where the cable of your internet provider "connects" to the internet. So that is the place google (and other websites) know because of technical reasons.
Of course, you may spoof this location by using VPNs or the tor network, but it's not said these means are per se more secure.

Personally I would much more worry about tracking by the fingerprint of data any browser sends. But I think this discussion is a bit off topic here

[orudge]

I for one have separate work and personal github accounts, registered to and committed to using work and personal email addresses respectively.
As long as you are non-abusive, work/individual identities is not likely to be a problem. Much of what is in ToSs is not enforceable or realistic anyway.

Microsoft employees tend to have GitHub accounts ending in "-msft". I'm sure many of them will have personal accounts too. Like you say, I don't expect it's a problem in reality.

[TrueBrain]

First of, as it goes, I do respect your view on this. Personally I disagree, and if you like, we can have lengthy debates about it But I do respect it, and understand where you are coming from.

Two things I want to add to the topic:

1) you are of course never bound to using GitHub. You are always free to, for example, use the forum to post your patches. Of course from an OpenTTD development point of view not optimal, but such is life Where OpenTTD's source is located should not hold you back of contributing to it If people like your stuff, it will find its way.
That said, I highly appreciate you making this topic and dumping your current work. That says a lot about you, and for that you earnt my respect.

2) GDPR (https://www.eugdpr.org/) is coming! Not sure it removes your issue towards GitHub, as GitHub is already very transparent in its doing, but if you live in the EU it is going to change a lot of that "sick feeling" we have about data protection. Of course it has to be seen how the implementation is going, but it basically forces companies to be a lot more transparent what they do with your data. You no longer have to guess or hope, but they have to tell you. The most important parts, for me personally, are "Right to Access" and "Right to be Forgotten" (see https://www.eugdpr.org/key-changes.html). And of course:

The conditions for consent have been strengthened, and companies will no longer be able to use long illegible terms and conditions full of legalese, as the request for consent must be given in an intelligible and easily accessible form, with the purpose for data processing attached to that consent. Consent must be clear and distinguishable from other matters and provided in an intelligible and easily accessible form, using clear and plain language. It must be as easy to withdraw consent as it is to give it.​

No more tricks / smoke and mirrors.

Of course this doesn't change anything about your argument that you can be tracked through your public profile. But it does make s*** Facebook pulled a lot harder (at least, that is the idea .. enforced by high penalties .. but .. yeah .. time will tell of course )

On that note, I really should figure out how OpenTTD is in relation to GDPR. I guess I should write something about that somewhere .. for example how long we store IPs and how they can be correlated etc. Although I think our ToS is fair and easy to read, I do have to check it complies with GDPR Fun stuff!