Skip to content

Conversation

MorphicDreamer
Copy link
Contributor

@MorphicDreamer MorphicDreamer commented Sep 8, 2021

Hey guys. I am adding religious city states to unciv. For now this is a draft as I am not done yet.

EDIT: ready for merge

this relates to issue #4290

@MorphicDreamer MorphicDreamer marked this pull request as draft September 8, 2021 08:00
@MorphicDreamer
Copy link
Contributor Author

does this game have artifacts?

@xlenstra
Copy link
Collaborator

xlenstra commented Sep 8, 2021

I'd advise on using the friendbonus and allybonus other city states also use, they can be found in eras.json

@xlenstra
Copy link
Collaborator

xlenstra commented Sep 8, 2021

does this game have artifacts?

What do you mean by artifacts?

@SomeTroglodyte
Copy link
Collaborator

And I'd even get a bonus for knowing such a city state when I'm at war with them...

This is expressly very early and unfinished, so I'll delay further critique. Welcome aboard!

@ravignir
Copy link
Contributor

ravignir commented Sep 8, 2021

not yet.

@xlenstra I guess he meant archaeology mechanic from bnw and the way antiquity sites work (created on city ruins/barb camps spots/tiles where battle was fought earlier in the game etc.). When you improve them with archaeologist you can dig up an artifact.

@SomeTroglodyte
Copy link
Collaborator

Or maybe they meant build artifacts? As in those expressly excluded from Android Studio?

@SimonCeder
Copy link
Collaborator

Thanks for the PR! As others said look at getStatMapForNextTurn() in CivInfoStats.kt. Since you've added the enum already, adding a couple lines to Eras.json should just work. You'll need to make sure religious city-states aren't generated in games with religion turned off, and also that religious city-states give some faith upon first contact.

@MorphicDreamer
Copy link
Contributor Author

@MorphicDreamer
Copy link
Contributor Author

@SomeTroglodyte Damn. Could have swore I put that if statement in

@SimonCeder
Copy link
Collaborator

No, we are not currently doing any BNW stuff

@MorphicDreamer
Copy link
Contributor Author

@SimonCeder okay

@MorphicDreamer
Copy link
Contributor Author

@SimonCeder yeah will work on all that you stated

@MorphicDreamer
Copy link
Contributor Author

@SimonCeder dumb question but what will the statmap accomplish

@xlenstra
Copy link
Collaborator

xlenstra commented Sep 8, 2021

@SimonCeder dumb question but what will the statmap accomplish

getStatMapForNextTurn() calculates the gold/science/culture/faith your civ will get at the end of your turn, split out into its contributing factors. The largest part of it is a for loop determining the stats gained from friendly/allied city states.

@MorphicDreamer
Copy link
Contributor Author

@SimonCeder ahh so you are saying refractor the code into that module got it

@SimonCeder
Copy link
Collaborator

Yes, you could put the code you wrote in the else clause together with the compatibility code for cultured CS, under the // Deprecated, assume Civ V values for compatibility. Otherwise the function should work as-is, you just need to add something like "Religious": ["Provides [3] [Faith] per turn"], to friendBonus and allyBonus in Eras.json

@MorphicDreamer
Copy link
Contributor Author

@SimonCeder okay

@SomeTroglodyte
Copy link
Collaborator

Damn. Could have swore I put that if statement in

🤣 I know the feeling, but then I'm already going 144... 🎂

@MorphicDreamer
Copy link
Contributor Author

@SimonCeder Thanks I don't really know Json's this well.

@MorphicDreamer
Copy link
Contributor Author

@SomeTroglodyte lol

@SomeTroglodyte
Copy link
Collaborator

Why is this giving me an error? when i run it on my laptop is works perfectly

"perfectly" in IDE and failing the github action is normal, as more build steps are run, among them the unit tests. Usually means you just don't see the "absolutely not perfectly" aspect by not testing enough. "Why" - go look and read the output. Threre's a "Details" link, you browse upwards, there's something like 2 of 60 unit tests failing, scroll some more, you'll see which and maybe some detail.

In this case, serializedLaziesTest FAILED makes the other test irrelevant. Sounds like You've added a by lazy into some class that gets serialized with a save game. You'll probably not really "work perfectly", just try saving a game. Background: Our serializer is written for Java and cannot cope with kotlin by lazy, so you have to help and tell it to ignore such a field with @delegate:Transient.

If you wish, you could try to learn how to run the same level of checks locally - see my scratchpad repo.

@MorphicDreamer
Copy link
Contributor Author

@SomeTroglodyte I did not use by lazy though. I think is must be something to do with the gameinfo class

@MorphicDreamer MorphicDreamer marked this pull request as draft September 10, 2021 13:31
@MorphicDreamer MorphicDreamer marked this pull request as ready for review September 10, 2021 13:31
@MorphicDreamer MorphicDreamer marked this pull request as draft September 10, 2021 13:31
@MorphicDreamer
Copy link
Contributor Author

There is no good way to find if city states are religious because
"lateinit property nation has not been initialized"
I think Yarim should make religious city states standard in the game somehow

@ajustsomebody
Copy link
Contributor

just make them not appear in the cs pool when starting a non religious game, i dont know how it is implemented in this pr but im guessing it directly tries to disable such city states

@MorphicDreamer
Copy link
Contributor Author

@logicminimal

There are 3 ways to determine if a city state is religous

  1. Determine if the name of the city state is religious. This is bad because if modders create their own religious city states, the game wouldn't know

  2. Determine if the type of city state is religious by looking at its class variables. Nations unfortunately gets initialised after the city state is set in stone and can't removed and replaced if religiousl

  3. I guess i could use the JSON. But that is kind of a pain and should only be used if no other solution is found

@SimonCeder
Copy link
Collaborator

@interdice
I've laid the groundwork for a simple way to exclude religious city-states in CityStateFunctions.kt, there's a TODO in initCityState. Just make a check for religious city states there, look at the check for militaristic above for reference. You might end up with something like

if (allPossibleBonuses.any { it.getPlaceholderText() == "Provides [] [] per turn" && it.getPlaceholderParameters()[1] == Stat.Faith.name}
            || (allPossibleBonuses.isEmpty() && cityStateType == CityStateType.Religious)) {

and then, if religion is disabled, return false and you're all set - no modifications to GameStarter.kt needed.

@MorphicDreamer MorphicDreamer marked this pull request as ready for review September 11, 2021 01:44
@MorphicDreamer
Copy link
Contributor Author

fixed


"declaringWar": "By god's grace we will not allow these atrocities to occur any longer. We declare war!",
"attacked": "May god have mercy on your evil soul.",
"defeated": "I for one welcome our new conquer overlord!",
Copy link
Owner

Choose a reason for hiding this comment

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

Cute :)

@yairm210
Copy link
Owner

Okay, I think this is finally ready!
Be aware that one this is released to users, there may be more things to solve, but that's just the way it is :)

@yairm210 yairm210 merged commit 2976187 into yairm210:master Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants