Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions libs/s25main/GlobalGameSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ void GlobalGameSettings::registerAllAddons()
AddonMilitaryAid,
AddonMilitaryControl,
AddonMilitaryHitpoints,
AddonMiningOverhaulCoal,
AddonMiningOverhaulGold,
AddonMiningOverhaulGranite,
AddonMiningOverhaulIron,
AddonMoreAnimals,
AddonNoAlliedPush,
AddonNoCoinsDefault,
Expand Down
52 changes: 52 additions & 0 deletions libs/s25main/addons/AddonMiningOverhaul.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
//
// SPDX-License-Identifier: GPL-2.0-or-later

#pragma once

#include "AddonList.h"
#include "mygettext/mygettext.h"

class AddonMiningOverhaulBase : public AddonList
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this lengthy name (here and in the name)? How would you translate it (to German)?

I mean:

  • Name: "Mining overhaul: Coal"
  • Description: "This addon lets you control mining behavior"

This is why I suggested "Mining behavior" thinking about how it will look in the (already long) list of addons with their currently active setting in the window: "Mining behavior (coal): No change" vs "Mining overhaul: Coal: No Change"

PS: Funny how the roles are reversed for once: You thinking like a developer, me like a user :D

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this lengthy name (here and in the name)? How would you translate it (to German)?

AddonZumAnpassenDerBergwerklogik :D

I'm not so happy with the behavior anymore. This somewhat more feels like something visible, possibly a worker. Like how does a soldier behave when his home building is lost, does he return, does he wander around. Overhaul implied some change of logic, which is often invisible and just like "physics", you feel the result but can't grab the reason itself - it's just the way it is. Can change the name if you don't agree to that though :)

"Mining behavior (coal): No change" vs "Mining overhaul: Coal: No Change"

I don't see where this Representation comes from, as there is no textual representation that way as values are highlighted by the right column. So I rather see Mining behavior (coal) vs Mining overhaul: coal where I liked the second version more, besides the discussion if it should be overhaul or behavior. Anyway, I don't care that much for the name in the window (except lenghty names like "Number of scouts required fore exploration expedition"). So same here, can change the name if you'd like me to :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where this Representation comes from, as there is no textual representation that way

I mean the Addon settings window. Currently it really reads (to me) like Addon title: Current setting. E.g. "Economy mode game length: 120min" and I kinda like that. I.e. a list that reads like plain English for every row so you understand it easily even when not reading the hint.

AddonZumAnpassenDerBergwerklogik :D

Ouch :D Seriously, I have that settings window in mind and try to imagine how it would look there.

So I rather see Mining behavior (coal) vs Mining overhaul: coal where I liked the second version more, besides the discussion if it should be overhaul or behavior.

I'd really like "behavior" better for a setting. Yes you are right with the description of "overhaul" but (at least to me) "overhaul" is the process of changing the behavior. I.e. reimplementing a part of the game to be "better" (performance, crash-resistance, ...). E.g. we need an overhaul of the save/load system as the current one sucks and crashes on big maps ;-)

As for the setting: This is something you change when selecting an option. And that option doesn't "overhaul" something but changes its "behavior".

Maybe we haven't found a good name yet. I mean what it changes is the behavior of the mines regarding resources. So maybe "Resource usage by mines: Coal". But that still doesn't fully hit it. It misses that the miner may produce nothing for a cycle. That's why I was asking for the German name. Just another way to think about it to come up with a nice flashy name for this :)

So I'd say come up with an English and German name here, maybe ask in Discord if anyone else has ideas, and check how it looks like in the Addon window before we finalize the name. So it is at least as good as possible in English and German ;-)

My current favourite is still "behavior", similar to what we have for the toolmaker: "Behavior [...]: ProduceNothing | ProduceRandom"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I may add my 2 cents:

First, on "mine" vs "mining":

  • "Mining overhaul", as the act of mining is being overhauled.
  • "Mine behavior", as behavior applies to the exhaustion of the mine.

Personally, I'd use "Mine exhaustion (Coal)"/"Minenerschöpfung (Kohle)". Also, note the parentheses. Otherwise, you'd end up with an awkward double-colon when writing out settings: "Mine exhaustion: Coal: Default"

An alternative might be "Mine productivity"/"Minenergiebigkeit". I don't like it but maybe it sparks someone else's idea.

{
protected:
AddonMiningOverhaulBase(AddonId addonId, const std::string& addonName)
: AddonList(addonId, AddonGroup::Economy, addonName,
_("This addon lets you control mining behavior.\n\n"
"No change: Original behavior\n"
"Settlers IV: Mines never deplete. Range is decreased to 1. Chance for production depends on "
"resource amount in range.\n"
"Inexhaustible: Mines never deplete\n"
"Everywhere: Mines never deplete, can mine everywhere"),
{
_("No change"),
_("Settlers IV"),
_("Inexhaustible"),
_("Everywhere"),
})
{}
};

class AddonMiningOverhaulCoal : public AddonMiningOverhaulBase
{
public:
AddonMiningOverhaulCoal() : AddonMiningOverhaulBase(AddonId::MINING_OVERHAUL_COAL, _("Mining overhaul: Coal")) {}
};

class AddonMiningOverhaulGold : public AddonMiningOverhaulBase
{
public:
AddonMiningOverhaulGold() : AddonMiningOverhaulBase(AddonId::MINING_OVERHAUL_GOLD, _("Mining overhaul: Gold")) {}
};

class AddonMiningOverhaulGranite : public AddonMiningOverhaulBase
{
public:
AddonMiningOverhaulGranite() : AddonMiningOverhaulBase(AddonId::MINING_OVERHAUL_GRANITE, _("Mining overhaul: Granite")) {}
};

class AddonMiningOverhaulIron : public AddonMiningOverhaulBase
{
public:
AddonMiningOverhaulIron() : AddonMiningOverhaulBase(AddonId::MINING_OVERHAUL_IRON, _("Mining overhaul: Iron")) {}
};
4 changes: 3 additions & 1 deletion libs/s25main/addons/Addons.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@
#include "addons/AddonToolOrdering.h"

#include "addons/AddonInexhaustibleFish.h"
#include "addons/AddonInexhaustibleGraniteMines.h"
#include "addons/AddonMaxRank.h"
#include "addons/AddonMilitaryAid.h"
#include "addons/AddonSeaAttack.h"

#include "addons/AddonInexhaustibleGraniteMines.h"
#include "addons/AddonMiningOverhaul.h"

#include "addons/AddonBattlefieldPromotion.h"
#include "addons/AddonBurnDuration.h"
#include "addons/AddonHalfCostMilEquip.h"
Expand Down
3 changes: 3 additions & 0 deletions libs/s25main/addons/const_addons.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ ENUM_WITH_STRING(AddonId, LIMIT_CATAPULTS = 0x00000000, INEXHAUSTIBLE_MINES = 0x

INEXHAUSTIBLE_GRANITEMINES = 0x00800000,

MINING_OVERHAUL_COAL = 0x00800001, MINING_OVERHAUL_GOLD = 0x00800002,
MINING_OVERHAUL_GRANITE = 0x00800003, MINING_OVERHAUL_IRON = 0x00800004,

MAX_RANK = 0x00900000, SEA_ATTACK = 0x00900001, INEXHAUSTIBLE_FISH = 0x00900002,
MORE_ANIMALS = 0x00900003, BURN_DURATION = 0x00900004, NO_ALLIED_PUSH = 0x00900005,
BATTLEFIELD_PROMOTION = 0x00900006, HALF_COST_MIL_EQUIP = 0x00900007, MILITARY_CONTROL = 0x00900008,
Expand Down
7 changes: 1 addition & 6 deletions libs/s25main/ai/aijh/AIPlayerJH.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -977,13 +977,8 @@ MapPoint AIPlayerJH::FindPositionForBuildingAround(BuildingType type, const MapP
foundPos = FindBestPosition(around, AIResource::Ironore, BuildingQuality::Mine, searchRadius);
break;
case BuildingType::GraniteMine:
if(!ggs.isEnabled(
AddonId::INEXHAUSTIBLE_GRANITEMINES)) // inexhaustible granite mines do not require granite
foundPos = FindBestPosition(around, AIResource::Granite, BuildingQuality::Mine, searchRadius);
else
foundPos = SimpleFindPosition(around, BuildingQuality::Mine, searchRadius);
foundPos = FindBestPosition(around, AIResource::Granite, BuildingQuality::Mine, searchRadius);
break;

case BuildingType::Fishery:
foundPos = FindBestPosition(around, AIResource::Fish, BUILDING_SIZE[type], searchRadius);
if(foundPos.isValid() && !ValidFishInRange(foundPos))
Expand Down
3 changes: 3 additions & 0 deletions libs/s25main/buildings/nobUsual.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ class nobUsual : public noBuilding
/// Wird gerade gearbeitet oder nicht?
bool is_working;

/// is this an empty cycle? (use wares but produce nothing)
bool is_emptyCycle;

~nobUsual() override;

void Serialize(SerializedGameData& sgd) const override;
Expand Down
100 changes: 91 additions & 9 deletions libs/s25main/figures/nofMiner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "network/GameClient.h"
#include "ogl/glArchivItem_Bitmap_Player.h"
#include "world/GameWorld.h"
#include <random/Random.h>

nofMiner::nofMiner(const MapPoint pos, const unsigned char player, nobUsual* workplace)
: nofWorkman(Job::Miner, pos, player, workplace)
Expand Down Expand Up @@ -71,20 +72,101 @@ helpers::OptionalEnum<GoodType> nofMiner::ProduceWare()

bool nofMiner::AreWaresAvailable() const
{
// FindPointWithResource triggeres outofresource message
if(GetAddonSetting() == 3)
return true;

return nofWorkman::AreWaresAvailable() && FindPointWithResource(GetRequiredResType()).isValid();
}

bool nofMiner::StartWorking()
unsigned int nofMiner::GetAddonSetting() const
{
MapPoint resPt = FindPointWithResource(GetRequiredResType());
if(!resPt.isValid())
return false;
const GlobalGameSettings& settings = world->GetGGS();
bool inexhaustibleRes = settings.isEnabled(AddonId::INEXHAUSTIBLE_MINES)
|| (workplace->GetBuildingType() == BuildingType::GraniteMine
&& settings.isEnabled(AddonId::INEXHAUSTIBLE_GRANITEMINES));
if(!inexhaustibleRes)
world->ReduceResource(resPt);

switch(workplace->GetBuildingType())
{
case BuildingType::GoldMine: return settings.getSelection(AddonId::MINING_OVERHAUL_GOLD);
case BuildingType::IronMine: return settings.getSelection(AddonId::MINING_OVERHAUL_IRON);
case BuildingType::CoalMine: return settings.getSelection(AddonId::MINING_OVERHAUL_COAL);
case BuildingType::GraniteMine: return settings.getSelection(AddonId::MINING_OVERHAUL_GRANITE);
default: return 0;
}
}

bool nofMiner::StartWorking()
{
workplace->is_emptyCycle = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a property of the workplace? Shouldn't this be part of the miner? You can use this in ProduceWare to return boost::none aka "nothing"


// needs to have at least one resource spot
// 0 = is exhaustible
// 1 = settlers IV like
// 2 = inexhaustible
// 3 = everywhere
unsigned int addonSettings = GetAddonSetting();

switch (addonSettings)
{
case 1: // settlers IV style
{
int sumResAmount = 0;
MapPoint useResPt;

std::vector<MapPoint> resPts = FindAllPointsWithResource(GetRequiredResType());

// iterate over all points
for each(MapPoint curPt in resPts)
{
// calculate the absolute amount of resource beneath
uint8_t resAmount = world->GetNode(curPt).resources.getAmount();
sumResAmount += resAmount;

// if there is one with more than 1 quantity, keep it (for reducing)
if(resAmount > 1 && !useResPt.isValid())
useResPt = curPt;
}

// no resources left (mine was built on invalid spot)
if(sumResAmount == 0)
return false;

// 19 = amount of nodes a mine can reach
// 7 = maximum resource amount a node possibly has
if(RANDOM.Rand(RANDOM_CONTEXT(), 19 * 7) > sumResAmount)
{
// failed, use food and start working - but produce nothing
workplace->is_emptyCycle = true;
//ProduceWare = boost::none;
} else
{
// if success, use 1 quantity if any
if(!useResPt.isValid())
world->ReduceResource(useResPt);
}
}
break;
case 2: // inexhaustible
{
MapPoint resPt = FindPointWithResource(GetRequiredResType());
if(!resPt.isValid())
return false;
}
break;
case 3: // inexhaustible, everywhere
{
}
break;
case 0: // original behavior
default:
{
MapPoint resPt = FindPointWithResource(GetRequiredResType());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
MapPoint resPt = FindPointWithResource(GetRequiredResType());
const MapPoint resPt = FindPointWithResource(GetRequiredResType());

if(!resPt.isValid())
return false;

world->ReduceResource(resPt);
}
break;
}

return nofWorkman::StartWorking();
}

Expand Down
3 changes: 2 additions & 1 deletion libs/s25main/figures/nofMiner.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
class SerializedGameData;
class nobUsual;

/// Klasse für den Schreiner
/// Klasse für den Miner
class nofMiner : public nofWorkman
{
protected:
Expand All @@ -23,6 +23,7 @@ class nofMiner : public nofWorkman
bool AreWaresAvailable() const override;
bool StartWorking() override;
ResourceType GetRequiredResType() const;
unsigned int GetAddonSetting() const;

public:
nofMiner(MapPoint pos, unsigned char player, nobUsual* workplace);
Expand Down
12 changes: 11 additions & 1 deletion libs/s25main/figures/nofWorkman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void nofWorkman::HandleStateWaiting2()
{
current_ev = nullptr;
// Ware erzeugen... (noch nicht "richtig"!, sondern nur viruell erstmal)
if(!(ware = ProduceWare()).has_value())
if(!(ware = ProduceWare()).has_value() || workplace->is_emptyCycle)
{
// Soll keine erzeugt werden --> wieder anfangen zu arbeiten
TryToWork();
Expand Down Expand Up @@ -110,3 +110,13 @@ MapPoint nofWorkman::FindPointWithResource(ResourceType type) const

return MapPoint::Invalid();
}

std::vector<MapPoint> nofWorkman::FindAllPointsWithResource(ResourceType type) const
{
// int maxResults = (int)((MINER_RADIUS * MINER_RADIUS + MINER_RADIUS) * 3u + 1u);

const std::vector<MapPoint> pts =
world->GetMatchingPointsInRadius<19>(pos, MINER_RADIUS, NodeHasResource(*world, type), true);

return pts;
}
3 changes: 3 additions & 0 deletions libs/s25main/figures/nofWorkman.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "nofBuildingWorker.h"
#include "gameTypes/GoodTypes.h"
#include "gameTypes/Resource.h"
#include <vector>
class SerializedGameData;
class nobBaseWarehouse;
class nobUsual;
Expand Down Expand Up @@ -38,6 +39,8 @@ class nofWorkman : public nofBuildingWorker

/// Looks for a point with a given resource on the node
MapPoint FindPointWithResource(ResourceType type) const;
/// Looks for all points with a given resource on the node
std::vector<MapPoint> FindAllPointsWithResource(ResourceType type) const;

public:
/// Going to workplace
Expand Down