Opened 13 years ago

Closed 12 years ago

Last modified 12 years ago

#786 closed enhancement (fixed)

[PATCH] Improve method for building walls

Reported by: Wolter Hellmund Owned by: vts
Priority: Must Have Milestone: Alpha 10
Component: Core engine Keywords: patch
Cc: Patch:

Description

Building walls can be a very tedious job if each wall section is built individually. Currently, this is the way walls are built in 0 a.d., which is a good enough reason for players not to build walls - the task is severely time-consuming.

Therefore, I propose that users are able to create path for walls to be built on. The placement of wall segments is something decided by the game engine.

The creation of these paths could be as simple as connecting points that the player selects, or as fancy as creating a bezier path along the ground to have a nice curvy wall. Note that a curvy wall is not only a luxury, but it can be a strategic building shape to hold a radius of terrain steadily.

Attachments (4)

786_walls_16apr12.patch (179.1 KB ) - added by vts 12 years ago.
WIP patch of the wall system. Start with -quickstart -autostart="WallTest" to play with it.
786_walls_04may12.patch (223.2 KB ) - added by vts 12 years ago.
786_walls_04may12.svn.patch (200.2 KB ) - added by historic_bruno 12 years ago.
Experiemental SVN version of the patch
logs.zip (10.7 KB ) - added by rogue-spectre 12 years ago.
log template problem

Download all attachments as: .zip

Change History (25)

comment:1 by vts, 12 years ago

Owner: set to vts
Status: newassigned

Currently looking into this.

See also http://www.wildfiregames.com/forum/index.php?showtopic=13684 for a discussion on how the walls should work.

comment:2 by FeXoR, 12 years ago

Plz see Ticket #1288 for the wall building system for random maps that include a linear wall placement function that may be used or copied to add walls in game/atlas.

The link http://www.wildfiregames.com/forum/index.php?showtopic=13684 does not seam to work so I'm not sure it works like you have in mind. However, see the prototype function 'placeLinearWall' of the class 'wallTool' for more information. I'd gladly help to add this as an in game feature.

comment:3 by Erik Johansson, 12 years ago

The link is to a topic in the private staff forums with design discussion. The in-game wall system will use a click+drag method to place the walls, so that's not necessarily relevant. There might still be parts of it that can be used though, so I'm sure vts is happy to see your comment :)

by vts, 12 years ago

Attachment: 786_walls_16apr12.patch added

WIP patch of the wall system. Start with -quickstart -autostart="WallTest" to play with it.

comment:4 by vts, 12 years ago

My current WIP patch, in git format. So far, it implements:

  • Click-and-drag wall placement.
  • Shift-click to continue building.
  • Snapping to existing towers or their foundations.

And more engine-wise:

  • Secondary obstruction control groups. These are necessary to allow walls to be built closer together to their surrounding towers than would otherwise be allowed by their obstruction shapes.
  • A whole bunch of tests for CmpObstructionManager and its various collision test filters.
  • The ability for PickSimilarFriendlyEntities to include foundations.
  • The ability for GetFoundationSnapData to radius-snap to a list of candidate entities.
  • A small demo map called WallTest to play around with the wall placement system.

Usual WIP disclaimer applies; this patch still contains temporary things for debugging, such as the celt fanatics on the testmap being able to build any of the wallsets. Start with -quickstart -autostart="WallTest" to play around with it.

Last edited 12 years ago by vts (previous) (diff)

comment:5 by Kieran P, 12 years ago

Keywords: patch review added
Milestone: BacklogAlpha 10
Type: defectenhancement

comment:6 by Kieran P, 12 years ago

Summary: Improve method for building walls[PATCH] Improve method for building walls

comment:7 by vts, 12 years ago

Summary: [PATCH] Improve method for building wallsImprove method for building walls

(This is still a WIP patch, not a final patch; it's intended just to show my progress so far for anyone who's interested.).

comment:8 by vts, 12 years ago

Keywords: wip added; review removed

comment:9 by michael, 12 years ago

What needs done in order for this to make Alpha 10, minus Gates? Gates can be an Alpha 11 or 12 enhancement.

Last edited 12 years ago by michael (previous) (diff)

in reply to:  9 comment:10 by vts, 12 years ago

Replying to michael:

What needs done in order for this to make Alpha 10, minus Gates? Gates can be an Alpha 11 or 12 enhancement.

I intend to commit the patch after some final tweaking of the wall parameters to ensure that a valid placement can be computed for every conceivable dragging distance. I was doing that last night, but couldn't get it done in time to commit it. Will try and finish it today or tomorrow.

Apart from that, I believe it should be quite usable, certainly enough for it to be committed and playtested. I think further enhancements and problem solving could then be done in additional tickets.

comment:11 by Kieran P, 12 years ago

Keywords: patch, wip → patch wip

Ah, I thought it might be a few days away, and didn't want to push it right in before feature freeze. Glad it'll be done for Alpha 10 :-)

One thing, is the code amount large enough and coupled with other parts of the system that it requires peer review? Expect/ask for possible improvements by other devs soon after committing it.

in reply to:  11 comment:12 by vts, 12 years ago

Replying to k776:

Ah, I thought it might be a few days away, and didn't want to push it right in before feature freeze. Glad it'll be done for Alpha 10 :-)

I think that, regardless of what we choose, we should allow for enough time for it to be playtested and validated so that we can be confident in it before releasing. Whether that means it will fit before A10, whether A10 will be pushed back a bit or whether it will need to be moved into A11 remains to be seen.

One thing, is the code amount large enough and coupled with other parts of the system that it requires peer review? Expect/ask for possible improvements by other devs soon after committing it.

There's a lot of new code, most of it on the JS side. I wouldn't say it's particularly tightly coupled, but there are parts with non-obvious logic that I expect to take a long time to thoroughly review though, because it requires a close understanding of how the wall system works, conceptually. It's certainly taken me several iterations to arrive at the current state. I've gone out of my way to document it as clearly as possible, but I still feel like I'd need to draw some diagrams and formulas to properly explain some things.

On the C++ side, the biggest change is some modifications to the obstruction manager, where I've added secondary obstruction control groups. I'm fairly confident in that part, since I've added extensive test cases for both the existing behaviour and the parts I've added.

There are some other things that I'd definitely like feedback on, however, such as the naming of things (WallSet and WallPiece components, ControlGroup2 vs SecondaryControlGroup, etc.). I'll definitely make sure to check in with the time at commit time.

by vts, 12 years ago

Attachment: 786_walls_04may12.patch added

comment:13 by vts, 12 years ago

Keywords: wip removed

Found and fixed a bunch of additional issues. For now, I'm attaching the patch to this ticket instead of committing it, with the intent of having some of the developers playtest it (particularly multiplayer, which is difficult to test locally for me). Once it has passed this smoke test, it should be ready-ish to be committed.

comment:14 by vts, 12 years ago

Summary: Improve method for building walls[PATCH] Improve method for building walls

by historic_bruno, 12 years ago

Attachment: 786_walls_04may12.svn.patch added

Experiemental SVN version of the patch

comment:15 by rogue-spectre, 12 years ago

Hi,

i'm tring to test this patch… so far i can do it. I get the latest source from the svn and apply the first patch (from git) and later i retry with the second patch from clean sources. I applied the patches using

patch -p0 -i 786_walls_04may12.svn.patch

When i create a basic military unit (default map and walltest) in order to be able to build a wall, there is no action avaiable to do with this unit and i get some errors,

WARNING: JavaScript warning: gui/session/utility_functions.js line 192 reference to undefined property template.cost

ERROR: JavaScript error: gui/session/utility_functions.js line 192 TypeError: template.cost is undefined getPopulationBonus([object Object])@gui/session/utility_functions.js:192 setupUnitPanel("Construction",[object Object],[object Object],[object Array],startBuildingPlacement)@gui/session/unit_commands.js:362 updateUnitCommands([object Object],[object GUIObject],[object GUIObject],[object Array])@gui/session/unit_commands.js:755 updateSelectionDetails()@gui/session/selection_details.js:274 onSimulationUpdate()@gui/session/session.js:322 onTick()@gui/session/session.js:216 __eventhandler40 (tick)([object Object])@sn tick:0

I've missed something to do ? I attache the log files from ~/.config/0ad/log. If you need more tell me.

Last edited 12 years ago by rogue-spectre (previous) (diff)

by rogue-spectre, 12 years ago

Attachment: logs.zip added

log template problem

comment:16 by vts, 12 years ago

Hi rogue-spectre,

The patch I've posted is a git patch, and the issue you see is caused by it not applying cleanly to an SVN working copy. Try using historicbruno's SVN version of the patch.

comment:17 by rogue-spectre, 12 years ago

i thought it might be a problem like that, so i get the git sources and apply the git patch to see if there was the same problem… and the same problem i've on the git sources. So i looked to the "utility_function.js" and temporarely modify

function getPopulationBonus(template)
{
	/*var popBonus = "";
	if (template.cost.populationBonus)
		popBonus = "\n[font=\"serif-bold-13\"]Population Bonus:[/font] " + template.cost.populationBonus;
	return popBonus;*/
        return 0;
}

One build later, i can finally try this patch, and it much much better than block by block construction, thank for your work ;). If you have things you want me to try and test for this patch, i'll be happy to help. So far i've see that there is some error when you try to drag wall outside your infunce area.

Thank you for this improvement ;)

comment:18 by vts, 12 years ago

Resolution: fixed
Status: assignedclosed

In 11760:

Wall placement. Closes #786.

in reply to:  17 comment:19 by vts, 12 years ago

Replying to rogue-spectre:

So far i've see that there is some error when you try to drag wall outside your infunce area.

I've committed my working version of the walls patch. Can you see if you still get this error? If so, can you elaborate on this a bit? What exactly is the error you're getting and how could I reproduce it?

Last edited 12 years ago by vts (previous) (diff)

comment:20 by rogue-spectre, 12 years ago

i've just updated to the latest source with your patch commited, the problem i had before is gone, i think it was related to the trick i made to make the patch work. I'll try further if i see something.

comment:21 by rogue-spectre, 12 years ago

So far it works great for stone wall. I tried the Roman civ only, and i noticed that wood wall construction is still by block and do not use your work.

Note: See TracTickets for help on using tickets.