[PATCH] Atlas Enhancement: Map re-sizing position

[michael]

In Atlas, I want to be able to choose from where to re-size a map--corners or center.

Attached image may make it clearer.

[Stephen Imhoff]

Initial new dialog entry

[Stephen Imhoff]

New dialog map growing

[Stephen Imhoff]

new dialog map shrinking

[Stephen Imhoff]

new dialog moving center

[Stephen Imhoff]

Added the ability to recenter the map, based on a mini-map like display (terrain only, though). This was done for one simple reason - under the original target design, there was no way to tell what "corner" was going to be chosen.

Note that the warning about resizing/recentering is very on-target, because it doesn't move any entities. This was previously mostly invisible due to always working from "upper-left", but since a new origin can be chosen the results may not be what is expected.

[elexis]

I noticed you use this repository ​https://github.com/Clockwork-Muse/0ad/commits/mapresize_tone (might make things easier to review)

[Stephen Imhoff]

Replying to elexis:

I noticed you use this repository ​https://github.com/Clockwork-Muse/0ad/commits/mapresize_tone (might make things easier to review)

Yeah, but I'm not sure how many people pay attention to the GitHub repository, given it's not the source of truth.

I keep debating about cloning the repository again and attempting a rebase/cleanup (ie, remove all historical instances of built binaries), but at this point I'd wait for the next version of premake, and get the new packaging feature. I'd probably end up with a list of commands to run, rather than a patch, to make it easier to review. That, and a set of instructions for artists since their workflow would most likely change. Probably the in-game assets are small enough Git wouldn't complain, it would just be the built executable they'd need to grab from somewhere else. Especially since I'd probably move most of the current stuff to it's own sub-repository, to make sure that it can be disabled cleanly.

But.... that's a bit offtopic for now.

[Stan]

To be honest I think only elexis does. I agree though it does make it easier to review patches of patches as you just have to make a branch. Now AtlasUI2 by trompetin17 was made on github and it's quite dead now.

On topic. Thanks for working on it Can you show us how it looks like ?

[Stephen Imhoff]

...I added images under attachments? Were those not enough?

[Stan]

Uh sorry yeah that functionnality looks great Did you make sure entities are deleted ? Looks good for commiting.

[Stephen Imhoff]

(could have sworn I commented on this earlier) No, I just left the existing functionality. So of course if there's anything in the map it gets moved around pretty badly. Looking into deleting, or recentering the units.

Amusingly, in testing, it looks like resizing the map didn't remove entities to begin with. It would blithely save entities off the edge of the map. Or, if they only ended up in the matted-out region, just not display them.

Should I just leave things as-is for now, and deal with entities in a separate patch, since it's already broken anyways?

[Michael D. Hafer]

Clockwork-Muse, this is fantastic work and outstrips my wildest imagination when I created this ticket 5 years ago.

I think the entity deletion is a crucial component of map resizing going forward and it's something that's always bothered me. Can't wait to see this added to Atlas.

[Stephen Imhoff]

Initial patch

[Stephen Imhoff]

Finally managed to spare some time to finish this. Entities are now deleted or moved, as appropriate. Note that, since the terrain data isn't saved for undo (but entities are), undoing may give some bizarre results (instant Atlantis!).

I still haven't updated the window string, since I'm not sure what translation/localization procedures are.

@Mythos_Ruler @elexis @stanislas69 @trompetin17

[Stan]

Atlas doesn't have localization yet.

Atlantis effect is bad though I believe it goes far beyond this ticket. So unless that's a trivial fix it should have its own patch.

[Stephen Imhoff]

Okay, should I just change the text, or is that something we have people managing in general (when you work for a big company that has people that manage text for consistency...)?

Not sure how trivial a change it would be. Potentially we could just make the action undoable, since the terrain data doesn't come back.

[Vladislav Belov]

About 1109_Recenter.patch: as I see, you use std::map<entity_id_t, CFixedVector3D>, but never search there, only store values, so you could use simple std::vector<std::pair<entity_id_t, CFixedVector3D>> instead, perfomance will be better.

Then m_NewPositions.emplace_back(entityId, position + offset) instead of m_NewPositions[entityId] = position + offset.

It's guranteed that entities have different IDs. Or even you have the same IDs there for some reason (really don't know why) you could sort + unique, that still faster than an insertion to a map.

[Stephen Imhoff]

Replying to vladislavbelov:

About 1109_Recenter.patch...

Done, But I'm going to avoid updating the patch for now, waiting to see if anybody else has something.

Replying to stanislas69:

Atlas doesn't have localization yet.

Atlantis effect is bad though I believe it goes far beyond this ticket. So unless that's a trivial fix it should have its own patch.

Took a quick look at the Atlantis problem as it effects this. The problem is that, during resize, we discard the old map data. So we have no idea what it was, and figuring out new heights is liable to be problematic (ie, you'd likely end up with civilizations on islands with no resources). Now, it looks possible to save off the data beforehand - we can get access to patches/tiles, and the underlying data. The problem is that we'd have to remove the cleanup from resize (since it deletes the previous set). So extra memory in use. Re-adding patches/tiles doesn't look too terrible, although it does create additional management overhead.

So I see a dichotomy here; mark the command as non-undoable (and update the text to warn) - possibly just for now - , or put in the extra effort to save off the map data too, and take the memory hit (an initial look suggests we might need three complete copies of the data - old, new, and current - instead of being able to toggle between old and new).

[elexis]

Feature freeze in 2 days.

[Stephen Imhoff]

Patch with true undo.

[Stephen Imhoff]

Actually, I couldn't find anything about turning undo off (thought I'd seen a flag at one point), so just made it work as true undo instead. Keeps both heightmaps and terrain tile data.

Dynamic data is cleaned up when the command goes off the stack - when a different command replaces it in history, new map is created, program is closed (at least).

[Vladislav Belov]

Few notes:

• &(dst.m_MiniPatches[0][0]) == &dst.m_MiniPatches[0][0], because priority of & is smaller than of ().
• m_SelectionRadius = double(std::min(m_NewSize, m_CurrentSize)) / std::max(m_NewSize, m_CurrentSize) * PanelRadius; why double?
• Few no new line warnings.

[elexis]

Moving to the new WIP milestone.

[Stephen Imhoff]

@vladislavbelov

• Hokay
• Because I needed a ratio, in the range of [0, 1). But I switched it to an integer-only operation.
• Fixed.

Sorry it took so long for me to notice. I thought notifications happened? Or am I just thinking of something else?

[Stan]

Replying to Stephen Imhoff:

@vladislavbelov

• Hokay
• Because I needed a ratio, in the range of [0, 1). But I switched it to an integer-only operation.
• Fixed.

Sorry it took so long for me to notice. I thought notifications happened? Or am I just thinking of something else?

I'm sorry it took us so much time to look at this ticket again. If you get this notification, would you please tell us if you are still interested in working on that patch ? If so, could you please port it to code.wildfiregames.com ? Our review process has changed and so have the tools, so no patch will get commited if they haven't been submitted to Phabricator first.see wiki:SubmittingPatches

[Stephen Imhoff]

​Differential 825

[Stan]

Thanks for keeping it updated :) and for coming back.

[Vladislav Belov]

Fixed in r23859:

Allow map to recenter during resize in Atlas. Fixes #1109.

Patch By: Clockwork-Muse

Tested By: Stan

Differential Revision: ​https://code.wildfiregames.com/D825