Author Topic: Adding Lava Related Code to OP2Helper library  (Read 5218 times)

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Adding Lava Related Code to OP2Helper library
« on: December 21, 2016, 12:07:12 PM »
Hey Everyone,

I think we should add some code to OP2Helper to ease programming lava/eruptions.

Mcshay wrote an interesting smart lava library but it appears there are only header files and a lava.lib file available so the source code isn't fully readable. There is some overlap in use though. http://forum.outpost2.net/index.php/topic,3208.msg53008.html#msg53008

Currently OP2Helper contains 2 structures in a header called lava.h that are designed to emulate how Dynamix programmers set Lava (but I don't know if they are used by anyone in practice?). I'm proposing expanding lava.h and adding lava.cpp. The new content would be functions easing the setting of large map sections as lava possible and easing setting or stopping lava flow animations. I would like to encapsulate the new code in the namespace Lava. The original 2 structures should probably remain outside the namespace for backward compatibility.

One function allows setting a MAP_RECT as lava possible. Another function allows setting any tile that has a cell type of S1 or S2 in a MAP_RECT as lava possible.

I pretty much dump identical code into whatever scenario I'm interested in adding a volcano to, so it would be helpful to me if it was contained in OP2Helper. While I made some minor edits, this code has been floating around the forums for a very long time and was not written by me.

If there is interest, I could write a function that searches out all tileIndices on a map that match the darker lava rock (that resides inside the gray rock) and set them as lava possible. Since I always set these tiles to S1 or S2 anyways, I'm not sure it is a needed function though.

Current lava.h contents:
Code: [Select]
// Note: This file contains code to emulate how official Sierra maps seem to be built.
// These structs are used to define data about where lava can expand on the map.

// Size: 0x8
struct Range
{
int start;
int end;
};

// Size: 0x1C  [0x1C = 28, or 7 dwords]
struct LavaPossibleInfo
{
int x;
Range y1;
Range y2;
Range y3;
};

Proposed addition to lava.h
Code: [Select]
#include "Outpost2DLL\Outpost2DLL.h"
#include "OP2Helper\OP2Helper.h"

namespace Lava
{
// Sets all tiles in a MAP_RECT to lava-possible
void SetLavaPossibleRegion(const MAP_RECT& mapRect);

// Sets all S1 and S2 Celltypes in a MAP_RECT to lava-possible
void SetLavaPossibleAllSlowCells(const MAP_RECT& mapRect);

//Functions to start the lava flow animation on the side of a volcano
void AnimateFlowSW(const LOCATION& TopLeftMostTile);
void AnimateFlowS(const LOCATION& TopLeftMostTile);
void AnimateFlowSE(const LOCATION& TopLeftMostTile);

//Functions to stop the lava flow animation on the side of a volcano
void FreezeFlowSW(const LOCATION& TopLeftMostTile);
void FreezeFlowS(const LOCATION& TopLeftMostTile);
void FreezeFlowSE(const LOCATION& TopLeftMostTile);
}

Proposed lava.cpp contents
Code: [Select]
namespace Lava
{
void SetLavaPossibleRegion(const MAP_RECT& mapRect)
{
int rectWidth = mapRect.x2 - mapRect.x1 + 1;
int rectHeight = mapRect.y2 - mapRect.y1 + 1;
int numberOfTiles = rectWidth * rectHeight;

LOCATION currentLoc;
for (int i = 0; i < numberOfTiles; ++i)
{
currentLoc = LOCATION(mapRect.x1 + i % rectWidth, mapRect.y1 + i / rectWidth);
GameMap::SetLavaPossible(currentLoc, true);
}
}

void SetLavaPossibleAllSlowCells(const MAP_RECT& mapRect)
{
int cellType;
LOCATION location;

for (int y = mapRect.y1; y <= mapRect.y2; ++y)
{
for (int x = mapRect.x1; x <= mapRect.x2; ++x)
{
location = LOCATION(x, y);
cellType = GameMap::GetCellType(location);

if (cellType == CellTypes::cellSlowPassible1 ||
cellType == CellTypes::cellSlowPassible2)
{
GameMap::SetLavaPossible(location, true);
}
}
}
}

void AnimateFlowSW(const LOCATION& loc)
{
GameMap::SetTile(loc + LOCATION(1, 0), 0x453);
GameMap::SetTile(loc, 0x447);
GameMap::SetTile(loc + LOCATION(0, 1), 0x45E);
GameMap::SetTile(loc + LOCATION(1, 1), 0x469);
}

void AnimateFlowS(const LOCATION& loc)
{
GameMap::SetTile(loc, 0x474);
GameMap::SetTile(loc + LOCATION(0, 1), 0x47E);
}

void AnimateFlowSE(const LOCATION& loc)
{
GameMap::SetTile(loc, 0x489);
GameMap::SetTile(loc + LOCATION(0, 1), 0x4A0);
GameMap::SetTile(loc + LOCATION(1, 1), 0x4AB);
GameMap::SetTile(loc + LOCATION(1, 0), 0x494);
}


void FreezeFlowSW(const LOCATION& loc)
{
GameMap::SetTile(loc + LOCATION(1, 0), 0x45A);
GameMap::SetTile(loc, 0x44F);
GameMap::SetTile(loc + LOCATION(0, 1), 0x465);
GameMap::SetTile(loc + LOCATION(1, 1), 0x470);
}

void FreezeFlowS(const LOCATION& loc)
{
GameMap::SetTile(loc, 0x47B);
GameMap::SetTile(loc + LOCATION(0, 1), 0x486);
}

void FreezeFlowSE(const LOCATION& loc)
{
GameMap::SetTile(loc, 0x490);
GameMap::SetTile(loc + LOCATION(0, 1), 0x4A8);
GameMap::SetTile(loc + LOCATION(1, 1), 0x4B2);
GameMap::SetTile(loc + LOCATION(1, 0), 0x49C);
}
}
« Last Edit: December 21, 2016, 12:23:20 PM by Vagabond »

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: Adding Lava Related Code to OP2Helper library
« Reply #1 on: December 22, 2016, 04:11:29 AM »
Quote
I'm proposing expanding lava.h and adding lava.cpp. The new content would be functions easing the setting of large map sections as lava possible and easing setting or stopping lava flow animations.

This is a great idea.

The proposed functions make sense.

I like the idea of wrapping things in namespaces, though my one concern here is that nothing else in the SDK is wrapped in a namespace, and so I'm hesitant to deviate from that.

Looking at the first function, a doubly nested loop probably makes more sense than using % and / to address the correct cell.

Offline leeor_net

  • Administrator
  • Hero Member
  • *****
  • Posts: 2350
  • OPHD Lead Developer
    • LairWorks Entertainment
Re: Adding Lava Related Code to OP2Helper library
« Reply #2 on: December 22, 2016, 09:14:10 AM »
Eh, I like the approach he took with it. I use a lot of this sort of math and it simplifies logic at the cost of somewhat obfuscating code behind math which for any reasonably good programmer shouldn't be too much of a problem.

I'm not sure of the performance gains/penalties with either approach... profiling would help here but at least for me the lack of a second loop makes perfect sense.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: Adding Lava Related Code to OP2Helper library
« Reply #3 on: December 22, 2016, 12:11:07 PM »
I wrote the function using the remainder operator to purposefully remove a nested loop. Someone else wrote the nested loop function on the forum that I shamelessly stole. It is a little odd that both approaches are used right next to each other.

When I have worked with 2D maps in the past, I've always preferred storing the map data in a 1D array as opposed to a 2D array. I think this made working with the remainder operator more natural than a nested x & y for loop. I also find working with a nested loop harder to read. But the remainder operator isn't any easier to read I suppose. So I guess what I'm saying is it doesn't matter too much to me which way it goes. It would be an easy function to re-write. Performance wise it doesn't matter much since you will only call the function once or twice at initialization to set lava possible areas.

If we remove the namespace from the code, will it be a problem if other custom scenarios have already defined their own function with the same name and reference OP2Helper? I know at least one other scenario uses very similar names, but I would have to check if they are identical or not. Unless there are other opinions to keep the namespace around, I'll remove it to keep the code base consistent.


EDIT

Below is SetLavaPossibleRegion re-written as a nested loop. Hooman is right, it looks cleaner than my approach, especially since the region is already packaged as a MAP_RECT.

Code: [Select]
for (int x = mapRect.x1; x <= mapRect.x2; ++x)
{
for (int y = mapRect.y1; y <= mapRect.y2; ++y)
{
GameMap::SetLavaPossible(LOCATION(x, y), true);
}
}



EDIT 2: This function needs to ensure that x1 and y1 are less then x2 and y2 unless MAP_RECT automatically does that.
« Last Edit: December 22, 2016, 09:53:59 PM by Vagabond »

Offline leeor_net

  • Administrator
  • Hero Member
  • *****
  • Posts: 2350
  • OPHD Lead Developer
    • LairWorks Entertainment
Re: Adding Lava Related Code to OP2Helper library
« Reply #4 on: December 22, 2016, 01:28:39 PM »
I would propose that we wrap the entire SDK into a namespace. It can be a bit painful as my experience doing that with NAS2D demonstrated but I think it will produce a cleaner output and will avoid name collisions as you mentioned. But I think that's a topic for a different discussion.

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: Adding Lava Related Code to OP2Helper library
« Reply #5 on: December 23, 2016, 03:29:24 AM »
Quote
Eh, I like the approach he took with it. I use a lot of this sort of math and it simplifies logic at the cost of somewhat obfuscating code behind math which for any reasonably good programmer shouldn't be too much of a problem.

Yes, I love the modulus operator. I would marry it in a heartbeat. But personal fetishes aside, it is technically less straight forward. A double nested loop is easily readable by anyone, including a novice programmer. A single loop using modulus, as glorious and foundational as it is, takes a bit of experience to understand.

It's also not the best fit for the problem. The DivMod code is useful to map a 1D index into a pair of 2D coordinates, while the reverse, multiplication and addition, can map 2D coordinates into a 1D index. Here you have 2D coordinates that will be mapped over a 2D range. Just stick with 2D.

Quote
I'm not sure of the performance gains/penalties with either approach... profiling would help here but at least for me the lack of a second loop makes perfect sense.

Quote
Performance wise it doesn't matter much since you will only call the function once or twice at initialization to set lava possible areas.

Correct, performance doesn't really matter here. You'd likely need a very large number of operations to measure any difference.

But, when there are details, I can never resist diving into them. In this case, the DivMod code is likely to be a bit slower. The modulus (%) and division (/) operations are relatively slow compared to other integer operations, such as increment (++). Further, the DivMod is done for every iteration of the inner loop, while in a doubly nested loop, the outer loop's increment is only done once for each full iteration of the inner loop.

One additional point to note is that at the assembly level, a single instruction (DIV) produces both the result for division (in EAX) and modulus (in EDX). The code as written may execute the same sequence of instructions twice, or an optimizing compiler could pick this up and reduce it down to a single operation and harvest both outputs (EAX and EDX registers) at the same time. Useful, since this instruction is the slow one.

But again, performance just isn't going to be an issue here. Another caveat, the above was theory, not profiling data.

Quote
It is a little odd that both approaches are used right next to each other.
Yes. ;)

Quote
If we remove the namespace from the code, will it be a problem if other custom scenarios have already defined their own function with the same name and reference OP2Helper?

Yes. Though if there are name collisions here, it's likely duplicate code, or a highly compatible alternate implementation. In which case the solution is to remove the duplicate code, which should probably be done anyway.

Quote
EDIT 2: This function needs to ensure that x1 and y1 are less then x2 and y2 unless MAP_RECT automatically does that.

The DivMod implementation has a similar issue. The numberOfTiles value could be negative. I don't think this is worth worrying about. It's generally understood the first coordinate is lower valued.

Quote
I would propose that we wrap the entire SDK into a namespace. It can be a bit painful as my experience doing that with NAS2D demonstrated but I think it will produce a cleaner output and will avoid name collisions as you mentioned. But I think that's a topic for a different discussion.

I agree, and would like to see things in a namespace too. Though I believe there was one showstopper on that idea. Namespaces affect mangled names, which means putting any imports from Outpost2.exe into a namespace will lead to link errors. In case I'm wrong here, try wrapping some Outpost2DLL code in a namespace and see what happens when you compile.

Granted, the namespacing is more of an issue for Outpost2DLL with direct imports from Outpost2.exe, as opposed to OP2Helper, which uses only indirect imports through Outpost2DLL. Guess I hadn't considered that before. Perhaps we should consider namespaces for OP2Helper and other projects outside of the Outpost2DLL project.

Offline Arklon

  • Administrator
  • Hero Member
  • *****
  • Posts: 1267
Re: Adding Lava Related Code to OP2Helper library
« Reply #6 on: December 23, 2016, 03:13:00 PM »
I agree, and would like to see things in a namespace too. Though I believe there was one showstopper on that idea. Namespaces affect mangled names, which means putting any imports from Outpost2.exe into a namespace will lead to link errors. In case I'm wrong here, try wrapping some Outpost2DLL code in a namespace and see what happens when you compile.
That is indeed the case. If it didn't affect name mangling, how could namespacing prevent name collisions?

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: Adding Lava Related Code to OP2Helper library
« Reply #7 on: December 24, 2016, 06:52:41 AM »
Hey everyone,

Thanks for the comments. I understand the name mangling issue with the namespace and that it will not ruin anything to wrap OP2 Helper functions and classes in namespaces. I'm not sure what EAX and EDX is though (I'm guessing related to assembly code?).

I'll plan to post a ver2 of the code in a little bit.

I did some test cases with MAP_RECT to see how it would handle wrapping issues with an East/West map and handling inverted values.

MAP_RECT accepts inverted values without crashing.

Code: [Select]
IE MAP_RECT(25, 25, 5, 5)

However, doing a check to see if a valid point is inside the MAP_RECT, it will return false.

Code: [Select]
IE MAP_RECT(25, 25, 5, 5);
// Will return false.
bool inside = mapRect.Check(LOCATION(15, 15);

When passing a MAP_RECT to wraps from East to West on a 512 tile width map or larger, check will pass if your MAP_RECT uses a smaller x2 value:

Code: [Select]
MAP_RECT mapRect(500 - 1, 0 - 1, 50 - 1, 20 - 1);

//Will return true on a 512 or greater size map
bool inside = mapRect.Check(LOCATION(5 - 1, 5 - 1));

Passing a x2 value greater than the width of the map that wraps east west will not pass check.

Code: [Select]
MAP_RECT mapRect2(500 - 1, 0 - 1, 600 - 1, 20 - 1);

// Fails
inside = mapRect2.Check(LOCATION(5 - 1, 5 - 1));

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: Adding Lava Related Code to OP2Helper library
« Reply #8 on: December 24, 2016, 08:25:52 AM »
Below is version 2 of the Lava code. Full header and cpp file are displayed. I left the namespace in and fixed the SetLavaPossibleRegion to a nested x, y loop.

Looking for other changes or more dissent if we really don't want the namespace to exist.

SetLavaPossibleRegion will not be able to handle horizontal wrapping on maps, but I think we have concluded that is okay.

Lava.h
Code: [Select]
#pragma once

#include "Outpost2DLL\Outpost2DLL.h"
#include "OP2Helper\OP2Helper.h"

namespace Lava
{
// Sets all tiles in a MAP_RECT to lava-possible.
void SetLavaPossibleRegion(const MAP_RECT& mapRect);

// Sets all S1 and S2 Celltypes in a MAP_RECT to lava-possible
void SetLavaPossibleAllSlowCells(const MAP_RECT& mapRect);

//Functions to start the lava flow animation on the side of a volcano
void AnimateFlowSW(const LOCATION& TopLeftMostTile);
void AnimateFlowS(const LOCATION& TopLeftMostTile);
void AnimateFlowSE(const LOCATION& TopLeftMostTile);

//Functions to stop the lava flow animation on the side of a volcano
void FreezeFlowSW(const LOCATION& TopLeftMostTile);
void FreezeFlowS(const LOCATION& TopLeftMostTile);
void FreezeFlowSE(const LOCATION& TopLeftMostTile);
}


// Note: The structs Range and LavaPossibleInfo emulate how official Sierra maps
//       define data about where lava can expand on the map.

// Size: 0x8
struct Range
{
int start;
int end;
};

// Size: 0x1C  [0x1C = 28, or 7 dwords]
struct LavaPossibleInfo
{
int x;
Range y1;
Range y2;
Range y3;
};

Lava.cpp
Code: [Select]
#include "Lava.h"

namespace Lava
{
void SetLavaPossibleRegion(const MAP_RECT& mapRect)
{
for (int x = mapRect.x1; x <= mapRect.x2; ++x)
{
for (int y = mapRect.y1; y <= mapRect.y2; ++y)
{
GameMap::SetLavaPossible(LOCATION(x, y), true);
}
}
}

void SetLavaPossibleAllSlowCells(const MAP_RECT& mapRect)
{
int cellType;
LOCATION location;

for (int y = mapRect.y1; y <= mapRect.y2; ++y)
{
for (int x = mapRect.x1; x <= mapRect.x2; ++x)
{
location = LOCATION(x, y);
cellType = GameMap::GetCellType(location);

if (cellType == CellTypes::cellSlowPassible1 ||
cellType == CellTypes::cellSlowPassible2)
{
GameMap::SetLavaPossible(location, true);
}
}
}
}


void AnimateFlowSW(const LOCATION& loc)
{
GameMap::SetTile(loc + LOCATION(1, 0), 0x453);
GameMap::SetTile(loc, 0x447);
GameMap::SetTile(loc + LOCATION(0, 1), 0x45E);
GameMap::SetTile(loc + LOCATION(1, 1), 0x469);
}

void AnimateFlowS(const LOCATION& loc)
{
GameMap::SetTile(loc, 0x474);
GameMap::SetTile(loc + LOCATION(0, 1), 0x47E);
}

void AnimateFlowSE(const LOCATION& loc)
{
GameMap::SetTile(loc, 0x489);
GameMap::SetTile(loc + LOCATION(0, 1), 0x4A0);
GameMap::SetTile(loc + LOCATION(1, 1), 0x4AB);
GameMap::SetTile(loc + LOCATION(1, 0), 0x494);
}


void FreezeFlowSW(const LOCATION& loc)
{
GameMap::SetTile(loc + LOCATION(1, 0), 0x45A);
GameMap::SetTile(loc, 0x44F);
GameMap::SetTile(loc + LOCATION(0, 1), 0x465);
GameMap::SetTile(loc + LOCATION(1, 1), 0x470);
}

void FreezeFlowS(const LOCATION& loc)
{
GameMap::SetTile(loc, 0x47B);
GameMap::SetTile(loc + LOCATION(0, 1), 0x486);
}

void FreezeFlowSE(const LOCATION& loc)
{
GameMap::SetTile(loc, 0x490);
GameMap::SetTile(loc + LOCATION(0, 1), 0x4A8);
GameMap::SetTile(loc + LOCATION(1, 1), 0x4B2);
GameMap::SetTile(loc + LOCATION(1, 0), 0x49C);
}
}

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: Adding Lava Related Code to OP2Helper library
« Reply #9 on: December 26, 2016, 04:51:22 AM »
Quote
That is indeed the case. If it didn't affect name mangling, how could namespacing prevent name collisions?

Well put. There'd be the potential for ambiguity if namespaces weren't part of the mangled names. That's certainly a good way to reason about it.

Though technically namespacing and name mangling are separate concerns. Within a compilation unit, there is no need for name mangling, even though namespaces may be used to resolve ambiguity. Name mangling is more of an implementation detail for linking. But that's being a bit pedantic.


Thank you Vagabond for posting those test results.


It would be nice for other people to weigh in on the namespacing topic. I'm still concerned about consistency. None of the other functions are namespaced. I like the idea of namespacing, but namespacing just one small section seems weird. Would it make more sense to just put everything in an OP2Helper namespace? I don't generally see libraries that have sub-namespaces, and I don't think this project is big enough to warrant that.

I think my recommendation here is to submit the code to the repository without the namespace, and then address the namespacing for the whole project as a separate issue. It probably makes the most sense to wrap the whole project in an OP2Helper namespace, or similar (suggestions?). Again though, project wide namespace changes should be a separate commit from the lava additions.

Offline leeor_net

  • Administrator
  • Hero Member
  • *****
  • Posts: 2350
  • OPHD Lead Developer
    • LairWorks Entertainment
Re: Adding Lava Related Code to OP2Helper library
« Reply #10 on: December 26, 2016, 10:16:24 PM »
I think I can agree with that. Makes sense to leave namespacing as a separate project/task.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: Adding Lava Related Code to OP2Helper library
« Reply #11 on: December 27, 2016, 12:07:14 AM »
I removed the namespace from Lava.h/Lava.cpp and committed to the repository.

In a separated commit, I removed the namespace from Bulldozer.h/.cpp that I had slipped in when creating the Bulldozer.h/.cpp files. If Dave is using OP2Helper Bulldozer functions in his new multiplayer scenarios, they will need to be updated by deleting the namespace reference. I'm not aware of anyone else who is using them...

In separate commits, I fixed my scenarios that had lava related code or were using the Bulldozer namespace. Since I had wrapped my own MapHelper functions in a namespace, there actually hadn't been any name collisions, so that was cool to realize. :D I still removed the code for the most part out of my scenarios.

I don't have a problem fragmenting even a smaller library into namespaces. It helps me track where each function/property belongs and group them together. I also understand that others might consider it overkill to put 8 or so functions in their own namespace.

Thanks for everyone's input on adding this code to the repository! Hopefully it will save someone a little time in the future when learning to program volcanoes.

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: Adding Lava Related Code to OP2Helper library
« Reply #12 on: December 27, 2016, 02:56:53 AM »
Thank you for doing this.

I noticed you've been pretty active with further developing the OP2Helper library.