Copy & Paste patch, reworked

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

Klaatu
Engineer
Engineer
Posts: 43
Joined: 04 Aug 2003 14:58

Re: Copy & Paste patch, reworked

Post by Klaatu »

Rubidium wrote:
Klaatu wrote:Bottom line, though: it is good to hear (for the first time, at least for me) that if the right implementation of a C&P type patch were to be written it would be considered for inclusion in trunk. Thanks.
Yup,
In the Suggestions forum. Never read it.
Rubidium wrote:never
In the General OpenTTD forum. Never read it.
Rubidium wrote:said
Not a C&P thread. Never read it.
Rubidium wrote:it.
Suggestions forum, also not a C&P specific thread. Never read it.
Rubidium wrote:Yup, never said it.
It's a good thing, then, that I never said you never said it. :)

That's also why I said
Klaatu wrote:...but in all other C&P threads I've read, and I think I've read them all...
Enough said? :roll:
CyberSoul
Engineer
Engineer
Posts: 1
Joined: 18 Mar 2011 03:03

Re: Copy & Paste patch, reworked

Post by CyberSoul »

First of all, I can confirm the code is messy and shouldn't be included in trunk in this form. That said, I was testing large PAX cells, which have 25+ copies of a station cell, and really wanted copy/paste. In the process, I updated this patch to trunk r22255, with no compiler warnings using gcc 4.4.4.

Also, road tunnels work now, and road bridges/tunnels that have both road and tram tracks copy/paste fine now. Stations and waypoints still don't work as stations were largely unimplemented even in the original code. Oddly, bridges fail to paste when rotated 180 degrees, but I didn't look into why as I don't need that feature at the moment.

Edit: The bridge rotation bug (which was present even before I updated it) got annoying, so I fixed it and updated the patch. Clicking a filename during save also properly updates the text now. Yikes, v2 was broken by a last minute change to clean up the code, so I changed that part back in v3.

Cheers,
CyberSoul
Attachments
CyberSoul_copypaste_r22255_v3.diff
Copy/Paste patch updated to r22255
(126.43 KiB) Downloaded 324 times
Nimda
Engineer
Engineer
Posts: 1
Joined: 01 Apr 2011 06:51

Re: Copy & Paste patch, reworked

Post by Nimda »

Hey guys,

can someone please upload a binary (win32) for this with the Version 1.0.5?

Thanks!
Alberth
OpenTTD Developer
OpenTTD Developer
Posts: 4763
Joined: 09 Sep 2007 05:03
Location: home

Re: Copy & Paste patch, reworked

Post by Alberth »

Nimda wrote:can someone please upload a binary (win32) for this with the Version 1.0.5
1.0.5 doesn't have the copy/paste patch. If you add it, it is not version 1.0.5 any more.
In other words, your request makes no sense.
Dante123
Chief Executive
Chief Executive
Posts: 671
Joined: 11 Aug 2009 16:10
Location: The Nederlands
Contact:

Re: Copy & Paste patch, reworked

Post by Dante123 »

idea for improvement on this patch:
calculated expense for what you are pasting at that moment.

in my game i have all expenses to max, but for most stuff i am pasting i dont know what they cost and when i have just the right amount to past it :)
ndh
Engineer
Engineer
Posts: 13
Joined: 30 Apr 2011 11:09

Re: Copy & Paste patch, reworked

Post by ndh »

Dante123, just hold Shift when Pasting. It gives an estimate, but it might not be very accurate.
ndh
Engineer
Engineer
Posts: 13
Joined: 30 Apr 2011 11:09

Re: Copy & Paste patch, reworked

Post by ndh »

Apart from the code quality, what exactly are the reasons this patch didn't make it into trunk?

I personally think it's trying to do too much (featuritis). What if the feature to save/load a template file would be dropped?
Rubidium
OpenTTD Developer
OpenTTD Developer
Posts: 3815
Joined: 09 Feb 2006 19:15

Re: Copy & Paste patch, reworked

Post by Rubidium »

In my opinion all issues seem to fall into the code quality category, ofcourse that category is quite big and the number of issues is big as well.
ndh
Engineer
Engineer
Posts: 13
Joined: 30 Apr 2011 11:09

Re: Copy & Paste patch, reworked

Post by ndh »

I agree, it's not very pretty right now. But before rolling up my sleeves I was just wondering if it's worth it. Looking at the patch, the multi-command approach seems questionable. I also don't like the way the copied area is stored, it seems quite hard to maintain. Then again, I'm not familiar with the existing code base, so those are just my first impressions.

Earlier in this thread, Alberth wrote
It depends on what you talk about. The current implementation versus the global idea. The former is really rejected, the latter has a few fundamental problems as you have read. If you want to make progress with the feature, I believe your time is better spent finding a solution for those problems
I was wondering exactly which problems he was referring to.
Eddi
Tycoon
Tycoon
Posts: 8267
Joined: 17 Jan 2007 00:14

Re: Copy & Paste patch, reworked

Post by Eddi »

the most "fundamental" problems of any copy-paste feature are (imho) the way to store data (in an upwards compatible way), and multiplayer-fairness issues.
Alberth
OpenTTD Developer
OpenTTD Developer
Posts: 4763
Joined: 09 Sep 2007 05:03
Location: home

Re: Copy & Paste patch, reworked

Post by Alberth »

ndh wrote:Earlier in this thread, Alberth wrote
It depends on what you talk about. The current implementation versus the global idea. The former is really rejected, the latter has a few fundamental problems as you have read. If you want to make progress with the feature, I believe your time is better spent finding a solution for those problems
I was wondering exactly which problems he was referring to.
That was a long time ago :)

I don't remember exactly. There are two or three threads about copy/paste patches that you should read I guess.
Personally, my biggest trouble is with load & save of pastes, as essentially, you can create and distribute known good solutions eg complete junctions or large stations, and everybody can just copy that file, and use those solutions without any effort or understanding of what they build.
Another fun part is probably multi-player support, as Eddi already mentioned.

However, both things have not been really discussed, since the code quality is so low. (Good quality of code is the #1 requirement, before anything else.)
Yexo
Tycoon
Tycoon
Posts: 3663
Joined: 20 Dec 2007 12:49

Re: Copy & Paste patch, reworked

Post by Yexo »

You also might want to contact adf88 too see how far he got with his last attempt. As for problems: read back in this very topic, especially page 7/8. See also those two posts. I am one of those listed there as "against the patch on game play principles". Although my opinion has changed somewhat over time, I'm still against a feature that lets you paste in big junctions without understanding them. The wiki has a large area with juntions. Unfortunately most of them are bad. Of course there are several ways to judge junctions, varying from efficiency to looks, but there are several that have neither of the two.
After playing PSG200 on the openttdcoop servers I'm convinced that sometimes copy/paste is very useful. See attached screenshot for part of that game.
Attachments
psg200-logic-island.png
psg200-logic-island.png (122.37 KiB) Viewed 787 times
User avatar
planetmaker
OpenTTD Developer
OpenTTD Developer
Posts: 9432
Joined: 07 Nov 2007 22:44
Location: Sol d

Re: Copy & Paste patch, reworked

Post by planetmaker »

Eddi wrote:the most "fundamental" problems of any copy-paste feature are (imho) the way to store data (in an upwards compatible way), and multiplayer-fairness issues.
Personally I would want for any c&p patch an adv. setting which has at least two, better three values for the c&p functionality:
c&p = [off | only own structures | all companies' structures] which would be a savegame setting, thus that servers can enforce it.
yorick
Engineer
Engineer
Posts: 80
Joined: 23 Mar 2008 08:53

Re: Copy & Paste patch, reworked

Post by yorick »

CyberSoul wrote:Also, road tunnels work now, and road bridges/tunnels that have both road and tram tracks copy/paste fine now.
Congratulations.

On breaking network-safety. (did you even try testing this on an unpatched server?)

Also it seems like a memory leak in ClearCommandQueue got introduced somewhen. Also the bug with pasting on the map edges which I fixed 2 (two!) years ago is still(again?) present. Also the tooltip for the convert rail button is still incomplete.
(Sorry, nothing personal, it's nice to see lots of people have tried to update this patch based on an old version, breaking it even further).
ndh
Engineer
Engineer
Posts: 13
Joined: 30 Apr 2011 11:09

Re: Copy & Paste patch, reworked

Post by ndh »

Alright, thank you all for the replies. Yexo, thank you for pointing out those two pages, I hadn't read them yet. I already had a look at adf88's patch, but there seems to be a lot of premature abstraction going on, which probably should not be part of a feature patch, and will most likely prevent inclusion into trunk. I believe adf88's patch has less features, but is already twice as big.

In my opinion, the main objective should be getting this feature ready for trunk first, then improve/extend it from there. Here's a list of features I consider essential. Without them being implemented flawlessly, this patch probably won't make it into trunk.

Core features
- Copy&paste railroads, roads, signals, depots. (Canals? Land purchases?)
- Copy&paste bridges and tunnels, but only if the entire object is contained within the selection.
- Shouldn't copy any stations or waypoints because they either couldn't be joined with other, preexisting stations, or the solution would be overcomplicated.
- Paste modifies the terrain to accomodate the copied layout. The top-most corners determines the overall level difference.
- Rotate a layout by 90, 180, or 270 degrees.
- Only copy items which belong to the player.
- Preview railroads, roads, tunnels, bridges before Paste.
- Shift+Paste displays a cost estimate instead of pasting.
- (?) Doesn't paste anything if the copied layout couldn't be reproduced completely (e.g. industry in the way, not enough money, parts would end up below sea level).
- A server-side option to disable Copy&paste, as some consider it cheating.

I'm not entirely sure about multi-player support, maybe at first this should only work in single-player mode, if only to calm everybody's minds. (And to get the naysayers hooked! :))

Additional features (can be added later, more important come first)
- (Optionally?) paste as much as possible if a complete paste isn't possible.
- Optionally modify the terrain in the entire pasted region, not just the tiles neccessary to reproduce the track layout.
- Mirror layout -> Problems: One way signals possibly shouldn't be mirrored.
- Copy diagonal region with Shift.
- Optionally toggle copying of roads/railroads[/canals].
- Convert rail type.
- Convert bridge types to max speed.
- Optionally copy from competitors. Must be a server setting.
- Save and load copied layouts to and from files -> Problems: File version management, upgrade old files to new version, unfairness in multi-player. Must be a server setting.

Problems I see with the current implementation:
- Pasting is implemented as a series of commands. I personally don't like this, but some might see it as a feature for multi-player games, because it's easier to limit what players can do with copy&paste. I'd rather see Paste implemented as just one command, but a CommandCallback only has 2 uint32's as custom parameter. Some additional magic would be required to transmit a copied layout over the network.

Alternatively, and this might be the simplest solution: Just store the area coordinates, not the contents. Paste would only work as long as the copied area hasn't been modified, but this is not necessarily a bad thing. It would also make it considerably harder to modify the patch to save/load templates to/from a file, which is probably a good thing, since it's the most controversial feature. It will make moving a layout by one tile harder though, since the copied and the pasted areas overlap.

- Instead of using existing data types (like Tile), it defines its own types, making it harder to maintain. It should probably use a bitmask of which parts of Tile it uses instead. I'm going to look at how the Demolish tool is implemented, as it's sort of similar to Paste in that it has to interact with different types of objects.
yorick
Engineer
Engineer
Posts: 80
Joined: 23 Mar 2008 08:53

Re: Copy & Paste patch, reworked

Post by yorick »

ndh wrote: Problems I see with the current implementation:
- Pasting is implemented as a series of commands. I personally don't like this, but some might see it as a feature for multi-player games, because it's easier to limit what players can do with copy&paste. I'd rather see Paste implemented as just one command, but a CommandCallback only has 2 uint32's as custom parameter. Some additional magic would be required to transmit a copied layout over the network.
One of the problems with the current implementation is also the maintainability. The DoCommands allow it to reuse the code for cost calculation and object placement, and also ensure that you can't do anything that you couldn't do manually. Removing the DoCommand chain would help with the server-side enforcements, and also look nicer, but it would require significantly more code and work.
User avatar
adf88
Chief Executive
Chief Executive
Posts: 644
Joined: 14 Jan 2008 15:51
Location: PL

Re: Copy & Paste patch, reworked

Post by adf88 »

I'm working on The Clipboard.

I took a new approach. Many problems you noticed will be resolved. Old approach builds a new logic on top of the OTTD. Now I'm trying to integrate as much as possible. I also resigned with compatibility with unpatched servers. Server maintainers should decide whether to allow to copy-pasting.

Firstly I generalized map accessors to work not only with the main map, but with other maps too - with contents of the the clipboard in this case. Basically I templatized the TileIndex to differentiate which map to operate on, i.e. instead of

Code: Select all

static inline uint TileHeight(TileIndex tile)
{
    assert(tile < MapSize());
    return GB(_m[tile].type_height, 0, 4);
} 
we have

Code: Select all

template <typename Tmap>
static inline uint TileHeight(TileIndexT<Tmap> tile)
{
    assert(tile < MapSize<Tmap>());
    return GB(Tmap::m[tile].type_height, 0, 4);
} 
Secondly I'm writing new command handlers. Similarly to TerraformTile_XXX functions I created a set of CopyPasteTile_XXX functions to operate on tiles. These handlers integrate with other handlers API. I'm also "upgrading" some existing handlers. Copy-pasting is a single command. In a network game it is possible to copy-paste instantly one piece of map into other location. In single play we can also copy to clipboard or paste from it.

in progress:
- Copy&paste railroads, roads, signals, depots. (Canals? Land purchases?):
NOTE: Cannals? Yes. Land purchases? Rather not. Surely not in multiplayer.
- Copy&paste bridges and tunnels, but only if the entire object is contained within the selection
- Paste modifies the terrain to accomodate the copied layout. The top-most corners determines the overall level difference.
NOTE: three modes - 1. full terraforming, 2. minimal terraforming (as much as needed), 3. no terraforming
- Rotate a layout by 90, 180, or 270 degrees.
NOTE: also reflecting is possible
- Only copy items which belong to the player.
- Shift+Paste displays a cost estimate instead of pasting.
NOTE: cost estimation will be quite precise
- (?) Doesn't paste anything if the copied layout couldn't be reproduced completely (e.g. industry in the way, not enough money, parts would end up below sea level).
- (Optionally?) paste as much as possible if a complete paste isn't possible.
NOTE: two modes: 1. paste as much as possible, 2. paste all or nothing (something near that)
- A server-side option to disable Copy&paste, as some consider it cheating.
NOTE: patch will be built in the way that without major rewrite (nearly writing a new implementation :) ) cheating will be impossible
- Optionally toggle copying of roads/railroads[/canals].
- Convert rail type.

planned (far feature):
- Preview railroads, roads, tunnels, bridges before Paste.
- Copy diagonal region with Shift.
NOTE: maybe, I'm not much convinced
- Save and load copied layouts to and from files -> Problems: File version management, upgrade old files to new version, unfairness in multi-player. Must be a server setting.
NOTE: It will not be possible in multiplayer mode, only single play. Besides this feature may be rejected because of the game philosophy.
- Convert bridge types to max speed.
NOTE: maybe, but only when save/loading is possible

other approach:
- Shouldn't copy any stations or waypoints because they either couldn't be joined with other, preexisting stations, or the solution would be overcomplicated
NOTE: that's not a problem, pasting will behave in the same like build a platform
- Optionally modify the terrain in the entire pasted region, not just the tiles neccessary to reproduce the track layout.
NOTE: only terraforming

rejected:
- Mirror layout -> Problems: One way signals possibly shouldn't be mirrored.
NOTE: Why not? They should.
- Optionally copy from competitors. Must be a server setting.
NOTE: No. No. No.

OK. I must go now. I will show the new code tomorrow (watch The Clipboard topic). Actually it is in Alpha stage. A lot works already.
Last edited by adf88 on 06 May 2011 07:21, edited 1 time in total.
:] don't worry, be happy and checkout my patches
Eddi
Tycoon
Tycoon
Posts: 8267
Joined: 17 Jan 2007 00:14

Re: Copy & Paste patch, reworked

Post by Eddi »

adf88 wrote:Firstly I generalized map accessors to work not only with the main map, but with other maps too - with contents of the the clipboard in this case. Basically I templatized the TileIndex to differentiate which map to operate on
i like that idea, it could also be used for non-surface map-tiles (e.g. flexible bridges/tunnels)
ndh
Engineer
Engineer
Posts: 13
Joined: 30 Apr 2011 11:09

Re: Copy & Paste patch, reworked

Post by ndh »

Adf88, I'm looking forward to seeing the code.
adf88 wrote:- Mirror layout -> Problems: One way signals possibly shouldn't be mirrored.
NOTE: Why not? They should.
Because when you mirror a drive-on-right layout on any axis it becomes a drive-on-left layout. But if you don't mirror signals, the "sidedness" stays the same. For example

Code: Select all

<<<<<<<
>>>>>>>
   v^
   v^
becomes

Code: Select all

>>>>>>>
<<<<<<<
   ^v
   ^v
Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 44 guests