Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some factions claim only one OMT #77428

Merged

Conversation

RenechCDDA
Copy link
Member

@RenechCDDA RenechCDDA commented Oct 29, 2024

Summary

Balance "Some factions claim only one OMT"

Purpose of change

Does not close the issue.

This was a particular problem for two of the Magiclysm factions, since they could spawn inside of cities and 'claim' a very wide area.

Describe the solution

Give factions a new json field, limited_area_claim, defaults false. Factions with this set to true will only care about the single OMT their camp(s) is on, instead of the entire reality bubble.

So if you encounter the Magiclysm magic shop guy, he only cares if you start breaking stuff in his shop. You can knock down the neighboring buildings without a complaint.

Gave it to the following factions:
Lapin, Grey Flame(Magicylsm, magic shop guy), The Ancient Ones(Magiclysm, wizard tower guy), and the dinomod swamper guys, since they occupy a single church tile.

Describe alternatives you've considered

We could store and track this on a camp-by-camp basis, but that would take more effort and I don't see any need for that right now.

Testing

Moved one tile outside of a magic shop's OMT, smashed. Was allowed to.
Moved one tile back into magic shop's OMT, smashed. Got the warning.

Additional context

Claimed area before and after:
cdda_fac_area_claim

@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding [C++] Changes (can be) made in C++. Previously named `Code` [Markdown] Markdown issues and PRs Mods: Magiclysm Anything to do with the Magiclysm mod Mods: Dinomod Anything to do with the Dinoclysm mod (DinoMod) Game: Balance Balancing of (existing) in-game features. json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Oct 29, 2024
@RenechCDDA
Copy link
Member Author

Auto review is still broken, so here's manual notifications.

@LyleSY for dinomod, @KorGgenT for Magiclysm

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Oct 29, 2024
@PatrikLundell
Copy link
Contributor

This seems a bit odd. A faction should claim the area owned by the faction, not whatever their members happen to see at the time. Thus, the occupied lumber mill survivor camp's members should claim the stuff inside their camp, but not the surrounding tiles, for example. It seems this is something that ought to be specified as part of the definition of the map special (or building, or whatever other OMT level type it is).

If you're going through with this, you should probably add the Scrapper to the set.

@RenechCDDA
Copy link
Member Author

This seems a bit odd. A faction should claim the area owned by the faction, not whatever their members happen to see at the time. Thus, the occupied lumber mill survivor camp's members should claim the stuff inside their camp, but not the surrounding tiles, for example. It seems this is something that ought to be specified as part of the definition of the map special (or building, or whatever other OMT level type it is).

If you're going through with this, you should probably add the Scrapper to the set.

I'm not sure we're on the same page here.

-This has nothing to do with line of sight.

-'claim' and 'owed' are synonyms here, it's just a matter of whether their claim/ownership extends the entire reality bubble or not

-The e.g. lumbermill still claims the entire reality bubble (as far as they can without some new method of delimiting their area)

If you see the example image, that's the grey flame faction 'healer's respite' from magiclysm. You can see how it is vexing and nonsensical that they would claim an entire city block despite occupying only one structure. Now they claim the structure they occupy, and nothing else.

There is potential for a faction which 'squats' on city buildings, but they can simply not set limited_area_claim

@PatrikLundell
Copy link
Contributor

I too was using "claim" and "own" as synonyms. Likewise my usage of "see" essentially means the same thing as the reality bubble (the lumber mill tends to have people stationed at the periphery).

I agree it's desirable to limit claims to what should be the faction's territory, but I'd prefer to see something that did that, rather than doing it only in one specific case. As far as I can see, this is a band aid to help with some particularly annoying cases, but it doesn't fix the underlying issue, so it will have to be fixed properly eventually (and that probably involves removing this band aid). One example it won't do anything about is a deserter area of 2*2 OMTs in a city. If they're hostile it probably doesn't matter, but if they aren't it does (I've never been close enough to them to find out if they are).
I agree city claims are particularly annoying, but I don't want to run afoul of the artisans for picking up some rocks or plants near their facility either.

@Procyonae
Copy link
Contributor

The deserters are hostile but I agree it would be better if this was more modifiable on a case by case basis by making it a mapgen definable zone (https://github.com/CleverRaven/Cataclysm-DDA/blob/master/doc/MAPGEN.md#place-a-zone-for-an-npc-faction-with-zones (I don't think this documentation is exhaustive)) for instance.

@PatrikLundell
Copy link
Contributor

A zone would be a possible way, although it may be more convoluted than necessary, given that you'd probably want to cover all of the faction's OMTs anyway (and zones are currently limited to being rectangular in shape). However, a flag in the definition of the site might well be implemented as a new type of zone.

@Maleclypse
Copy link
Member

Error: /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/faction_camp.cpp:691:18: error: static member accessed through instance [readability-static-accessed-through-instance,-warnings-as-errors]
  691 |         if( p == it.npos ) {
      |                  ^~~
      |                  std::basic_string<char, std::char_traits<char>, std::allocator<char>>::

Clang tidy error

@RenechCDDA
Copy link
Member Author

Unrelated and was resolved by #77420

https://github.com/CleverRaven/Cataclysm-DDA/pull/77420/files#diff-f0de892fdc66c1d8d3a18d71a9f8263c954b66a51c62f5ac024b84f295c68640L691-R691

@Maleclypse Maleclypse merged commit abd7e37 into CleverRaven:master Nov 2, 2024
22 of 30 checks passed
@RenechCDDA RenechCDDA deleted the faction_limited_area_claim branch November 3, 2024 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` <Documentation> Design documents, internal info, guides and help. Game: Balance Balancing of (existing) in-game features. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs Mods: Dinomod Anything to do with the Dinoclysm mod (DinoMod) Mods: Magiclysm Anything to do with the Magiclysm mod Mods Issues related to mods or modding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants