Opened 18 years ago

Closed 10 years ago

Last modified 10 years ago

#96 closed enhancement (fixed)

[PATCH][Atlas] Copy-and-paste of entities

Reported by: Philip Taylor Owned by: trompetin17
Priority: Should Have Milestone: Alpha 17
Component: Atlas editor Keywords: patch
Cc: Patch:

Description

Allow copy-and-paste of various things, like the selected objects.

Probably implement by converting the data to XML and saving as text into the clipboard, so that people can easily edit it in a text editor or save it to disk and then copy it back into Atlas.

Attachments (7)

copy_example.diff (2.5 KB ) - added by trompetin17 10 years ago.
copypaste.diff (17.4 KB ) - added by stilz_ 10 years ago.
progress_1_96.diff (7.5 KB ) - added by trompetin17 10 years ago.
progress_1_92_II.patch (12.1 KB ) - added by trompetin17 10 years ago.
Fix
progress_1_96_III.patch (14.1 KB ) - added by trompetin17 10 years ago.
Fixed
progress_1_96_IV.patch (14.5 KB ) - added by trompetin17 10 years ago.
Finally?
progress_1_96_V.patch (16.9 KB ) - added by trompetin17 10 years ago.

Download all attachments as: .zip

Change History (48)

comment:1 by Jason, 18 years ago

Not sure if its related, but a copy and paste from one window of the actor editor to another would be handy. :brow:

comment:2 by (none), 13 years ago

Milestone: Atlas Stage 1

Milestone Atlas Stage 1 deleted

comment:3 by Andrew, 13 years ago

Milestone: Backlog

comment:4 by dee@earlsoft.co.uk, 13 years ago

Windows (at least) allows multiple data types to be stored on the clipboard so you can have your own binary format as well as a human readable format, image, etc...

comment:5 by Kieran P, 13 years ago

Milestone: BacklogAlpha 4
Owner: Philip Taylor removed
Type: taskenhancement

comment:6 by Kieran P, 13 years ago

Summary: Copy-and-paste[Atlas] Copy-and-paste of entities

comment:7 by Kieran P, 13 years ago

Milestone: Alpha 4Alpha 5

comment:8 by Kieran P, 13 years ago

Milestone: Alpha 5Alpha 6
Priority: Nice to HaveIf Time Permits

comment:9 by Kieran P, 13 years ago

Milestone: Alpha 6Backlog

comment:10 by stilz_, 10 years ago

Owner: set to stilz_
Status: newassigned

comment:11 by leper, 10 years ago

Owner: stilz_ removed
Priority: If Time PermitsShould Have
Status: assignednew

#1212 was a duplicate of this (and has IMO the better description of what should be implemented).

comment:12 by trompetin17, 10 years ago

Do we have any cross-platform clipboard in the project?

EDIT: #include "lib/sysdep/clipboard.h"

Last edited 10 years ago by trompetin17 (previous) (diff)

by trompetin17, 10 years ago

Attachment: copy_example.diff added

comment:13 by trompetin17, 10 years ago

Keywords: review patch added
Milestone: BacklogAlpha 17
Summary: [Atlas] Copy-and-paste of entities[PATH][Atlas] Copy-and-paste of entities

comment:14 by trompetin17, 10 years ago

I added copy example, could you review if it is the right way please.

comment:15 by sanderd17, 10 years ago

I had to rebuild my workspaces (didn't expect that), but after that, it didn't segfault anymore.

When pressing CTRL+C, I saw this output in my terminal:

sys_clipboard_set: <AIProxy></AIProxy><Footprint><Circle radius="2.5"></Circle><Height>15.0</Height></Footprint><Identity><Civ>gaia</Civ><Classes datatype="tokens">ForestPlant</Classes><GenericName>Tree</GenericName><History>Pliny narrates several remarkable, but not incredible, instances of the durability of Cypress-wood. He says that there were in his time Cypresses still standing at Rome which were more ancient than the city itself; but that the tree was not a native of Italy, having been originally introduced from Greece to the Greek colony of Tarentum; whence, indeed, Cato, in his work on "Rural Economy," recommends that its seed should be procured. The doors of the temple of Diana, at Ephesus, were, Pliny relates, of Cypress-wood, and appeared quite new when four centuries old; as did also the statue of Jupiter in the Capitol, which was of the same material and half as old again. The tree in his time was employed for rafters, joists, and especially for vine-props, so that a Cypress grove was thought a valuable dowry for a daughter.</History><Icon>gaia/flora_tree_generic.png</Icon><SpecificName>Cypress Tree</SpecificName><Tooltip>Chop down to accumulate Wood.</Tooltip></Identity><Minimap><Colour b="68" g="107" r="69"></Colour><Type>wood</Type></Minimap><Obstruction><Active>true</Active><BlockConstruction>true</BlockConstruction><BlockFoundation>true</BlockFoundation><BlockMovement>true</BlockMovement><BlockPathfinding>true</BlockPathfinding><DisableBlockMovement>false</DisableBlockMovement><DisableBlockPathfinding>false</DisableBlockPathfinding><Unit radius="1.5"></Unit></Obstruction><OverlayRenderer></OverlayRenderer><Ownership></Ownership><Position><Altitude>0</Altitude><Anchor>upright</Anchor><Floating>false</Floating><TurnRate>6.0</TurnRate></Position><ResourceSupply><Amount>200</Amount><KillBeforeGather>false</KillBeforeGather><MaxGatherers>8</MaxGatherers><Type>wood.tree</Type></ResourceSupply><Selectable><Overlay><Texture><MainTexture>circle/128x128.png</MainTexture><MainTextureMask>circle/128x128_mask.png</MainTextureMask></Texture></Overlay></Selectable><Sound><SoundGroups><select>interface/select/resource/sel_tree.xml</select></SoundGroups></Sound><StatusBars><BarHeight>0.5</BarHeight><BarWidth>3.0</BarWidth><HeightOffset>10.0</HeightOffset></StatusBars><Vision><AlwaysVisible>false</AlwaysVisible><Range>0</Range><RetainInFog>true</RetainInFog></Vision><VisualActor><Actor>flora/trees/mediterranean_cypress.xml</Actor><SelectionShape><Footprint></Footprint></SelectionShape><SilhouetteDisplay>false</SilhouetteDisplay><SilhouetteOccluder>true</SilhouetteOccluder><VisibleInAtlasOnly>false</VisibleInAtlasOnly></VisualActor>

But it seems to not be on my clipboard (I see the paste command isn't implemented in Atlas, but I also can't paste it to a text editor).

Also, you're putting the wrong stuff in the clipboard. It should be the template name instead of the complete template, accompanied with extra position and variation info. This combination of info also gets written to XML when writing the map file (just open a map xml file and scroll down a bit until you see all entity definitions).

In any way, there are two things that certainly need to be in the clipboard: template name and entity rotation. The entity position should depend on the cursor position when pasting. The variation may or may not be copied from one to the other.

comment:16 by stilz_, 10 years ago

I'm just adding my diff in case it would be useful. My approach was not to copy only template name and rotation but all properties - so everything is preserved.

by stilz_, 10 years ago

Attachment: copypaste.diff added

comment:17 by historic_bruno, 10 years ago

Atlas uses wxWidgets, IMO it's best to start with what wxWidgets can do, and it actually has a wxClipboard class :) It likely also has XML parsing and construction, if needed.

I think for now, sanderd17 is correct, the full template shouldn't be copied - what could we do with it? AFAIK there would be no way to insert modified template data back into the simulation via a "paste". (It could even be argued whether we should use XML or JSON for this)

I do like the idea of using the clipboard, rather than internal copy/paste, because as the ticket description says, the data can be modified outside of Atlas (maybe by script/sed) and then pasted back. It's a flexible approach.

It should support multiple object selections, by the way. If not, the user won't be clear on which object will be copied when multiple are selected. That may require modifying Atlas' selected object data structure, as I recall it's inadequate for other things.

comment:18 by Stan, 10 years ago

It´d be nice if you could save those copy paste. Let´s say you have a template bridge right and bridgeleft you copy them and when you make a new map you just have to import them. :)

by trompetin17, 10 years ago

Attachment: progress_1_96.diff added

comment:19 by trompetin17, 10 years ago

Adding "Copy selected object". when you select one or more entities inside map you can press Ctrl+c, then you can copy in another tool(notepad?). you will get something like

<Entities>
 <Entity>
  <template></template>
  <position x="" z=""></position>
  <rotation></rotation>
  <player></player>
  <actorseed></actorseed>
 <Entity>
 ....
</Entities>

"Paste" is partially implement, im trying to figure it out how to "show" more the one preview object, (sander give some ideas today, i will try to get it tonigth).

Last edited 10 years ago by sanderd17 (previous) (diff)

comment:20 by Stan, 10 years ago

Could you also copy terrain layout and terrain height ? And make the user able to save them as patterns ? (I can developp on that if you need)

comment:21 by sanderd17, 10 years ago

Copying terrain isn't even slightly related to this job. The terrain structure, the interface to modify it, the storage and all is completely different from entities.

in reply to:  21 comment:22 by historic_bruno, 10 years ago

Replying to sanderd17:

Copying terrain isn't even slightly related to this job.

It can go in #92, if anyone wants to work on that.

comment:23 by stilz_, 10 years ago

@trompetin17: Preview for multiple entities at once is implemented in my patch, you can look there.

comment:24 by trompetin17, 10 years ago

@stilz_: i saw what your patch have, but the method ObjectPreview, delete the pre object preview, so when you add the second object the first one is delete, and you dont pach ObjectPreview(at least in your diff i cant see it)

comment:25 by historic_bruno, 10 years ago

Owner: set to trompetin17

comment:26 by historic_bruno, 10 years ago

Keywords: wip added

comment:27 by leper, 10 years ago

Summary: [PATH][Atlas] Copy-and-paste of entities[PATCH][Atlas] Copy-and-paste of entities

comment:28 by sanderd17, 10 years ago

Any progress on this patch? It would be a great addition to Atlas.

comment:29 by trompetin17, 10 years ago

Hi this is my update, you can:

  • create object
  • select object
  • press ctrl+c
  • press ctrl+v
  • move the mouse till you get the position you want
  • if you dont wanna paste press esc
  • with left mouse button you can create the object(partially implementend)

by trompetin17, 10 years ago

Attachment: progress_1_92_II.patch added

Fix

comment:30 by trompetin17, 10 years ago

Ok my work is done. please review and tell me if need to fix something :D

by trompetin17, 10 years ago

Attachment: progress_1_96_III.patch added

Fixed

comment:31 by sanderd17, 10 years ago

The code works very well, but some remarks on the name/style:

  • You put the Template XML node in a string called objectId. In most of our code, the ID stands for a number referring to the actual entity. So you should call it template, or even better, TemplateName.
  • We also don't use the name "object" to refer to game objects. We call them "entity". So instead of objectPos, entityPos would be better. Same for m_ObjPosition, which could be m_EntPosition.
  • We try to avoid unneeded braces. So whenever an "if" has only one operation attached, you can drop the braces.
  • The word "pivot" (in the MoveObjectPreview) is normally used when it's related to rotating something. Here you're only moving stuff. So "pivot" isn't correct. A better name would be "originalPosition" or "referencePosition". You don't have to care about floating (can always consider it true), as we can copy-paste floating and non-floating entities in one batch.
  • Try to reduce the amount of conversions between Fixed and Float in your MoveObjectPreview. Make sure your "dir" is a CFixedVector3D, and most other conversions should be gone.

Apart from those remarks, your patch looks fine to me. But I'd also like historic_bruno to give a review.

Nice extra features would be undo and redo actions. It would also be nice if you could automatically select all pasted entities when pasting. So after pasting, you can still adjust the position a bit. And when we have rotation of multiple entities, it should make it possible to easily rotate your just pasted entities.

by trompetin17, 10 years ago

Attachment: progress_1_96_IV.patch added

Finally?

comment:32 by trompetin17, 10 years ago

it just missing undo behaviour,

  • Added "automatically select all pasted entities when pasting"
Last edited 10 years ago by trompetin17 (previous) (diff)

comment:33 by sanderd17, 10 years ago

There are some problems with this patch.

First, the paste doesn't work if there are still selected objects. Due to the

if (!g_SelectedObjects.empty())
{
	if (evt.GetKeyCode() == 'C')
	{
		// ...
		return true;
	}
}
else if (evt.GetKeyCode() == 'V')
{
	obj->OnPasteStart();
	return true;
}

structure.

This breaks the fast select -> copy -> paste workflow, and adds an unneeded click.

When I removed the "else", I also saw that the original selection isn't cleared when pasting. IMO, when pasting, the original selection should be cleared, so when a user hits "delete" because his paste was wrong, or when he moves his paste, he shouldn't delete the original. Apparently, g_SelectedObjects.clear() instead of g_SelectedObjects.empty() works.

A second problem I saw is that the ownerships aren't saved. Instead, the preview ownership gets the ownership set in Atlas (so the one selected in the list, or the ownership from the last selected object), while IMO, it should be the ownership of the original elements.

This probably requires a extra parameter to the ObjectPreview message. Set to -1 if it needs to take the atlas ownership, and set to the correct player id if it needs to take a different ownership.

And a last problem I noticed, the paste doesn't appear on pressing CTRL+V, but only after moving your mouse when you pressed the CTRL+V. This should also be solved.

Last edited 10 years ago by sanderd17 (previous) (diff)

by trompetin17, 10 years ago

Attachment: progress_1_96_V.patch added

in reply to:  33 comment:34 by trompetin17, 10 years ago

Fixed both problems

Replying to sanderd17:

There are some problems with this patch.

First, the paste doesn't work if there are still selected objects. Due to the

if (!g_SelectedObjects.empty())
{
	if (evt.GetKeyCode() == 'C')
	{
		// ...
		return true;
	}
}
else if (evt.GetKeyCode() == 'V')
{
	obj->OnPasteStart();
	return true;
}

structure.

This breaks the fast select -> copy -> paste workflow, and adds an unneeded click.

When I removed the "else", I also saw that the original selection isn't cleared when pasting. IMO, when pasting, the original selection should be cleared, so when a user hits "delete" because his paste was wrong, or when he moves his paste, he shouldn't delete the original. Apparently, g_SelectedObjects.clear() instead of g_SelectedObjects.empty() works.

A second problem I saw is that the ownerships aren't saved. Instead, the preview ownership gets the ownership set in Atlas (so the one selected in the list, or the ownership from the last selected object), while IMO, it should be the ownership of the original elements.

This probably requires a extra parameter to the ObjectPreview message. Set to -1 if it needs to take the atlas ownership, and set to the correct player id if it needs to take a different ownership.

And a last problem I noticed, the paste doesn't appear on pressing CTRL+V, but only after moving your mouse when you pressed the CTRL+V. This should also be solved.

comment:35 by sanderd17, 10 years ago

Resolution: fixed
Status: newclosed

In 15381:

Add copy-paste (CTRL+C/V) to Atlas. Patch by trompetin17. Fixes #96

comment:36 by sanderd17, 10 years ago

Keywords: review wip removed

comment:37 by ben, 10 years ago

In 15415:

Fixes MSVC warning about uninitialized playerId in Atlas entity pasting code (using 0 as a sensible default value), refs #96

comment:38 by historic_bruno, 10 years ago

Fixed a build warning, but it's not unlikely that we will want some error handling and sanity checking in the pasting code. For example, wxString::ToLong might fail on an invalid number but we ignore it's return value. That's not for this ticket though. Thanks for all your work on this! :)

comment:39 by Erik Johansson, 10 years ago

Not sure if it's best to add a new ticket or open this ticket, so I'll just comment to being with: It would be nice to have Copy (Ctrl + C), and Paste (Ctrl + V) appear in the Edit menu to help people discover that the option exists.

comment:40 by historic_bruno, 10 years ago

Please make new tickets for copy-paste improvements/fixes, this one is old enough :)

comment:41 by Erik Johansson, 10 years ago

Cool, just thought I'd ask first before making a ticket for something potentially too small :) I have no idea how much work it is/isn't to implement the above :)

Note: See TracTickets for help on using tickets.