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

Georgia #267

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Georgia #267

wants to merge 3 commits into from

Conversation

DeStup
Copy link
Contributor

@DeStup DeStup commented Feb 12, 2025

No description provided.

@DeStup
Copy link
Contributor Author

DeStup commented Feb 12, 2025

I know that a key unique ability hasn't been realized. But in mod are already nations in fashion that suffer from this, like Georgia.

"[+2 Production] from every [Pagoda]",
"[+2 Production] from every [Synagogue]",
"[+2 Production] from every [Vihara]"
// TODO: new uniq Buildings unlocked by religious beliefs (except Houses of Worship) are built using Production instead (cost equal to 50% of the required Faith)
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should be unique buildings, not a proper UA. UA should be replaced with a comment mentioning this. I think(?) the buildings shouldn't be displayed in the civilopedia

Copy link
Contributor Author

@DeStup DeStup Feb 13, 2025

Choose a reason for hiding this comment

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

The problems here:

  • If we add "replace" and "hidden for civilopedia" for uniq building, then link in the civilopedia's Cathedral(vanilla) breaks.
    It can still be bought for faith..

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can still be bought with faith would be a problem here regardless, so I'm actually not concerned about that here. Also, that can be dealt with

Also, I'm pretty sure it doesn't break the link for the original cathedral. It would only break the link in the cathedral's entry that links to the replacement, which is... fine. The point of making these hidden from the civilopedia is to avoid their nation entry from showing a ton of buildings that's not actually particularly relevant (they don't do anything particularly noteworthy besides being buildable)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can still be bought with faith would be a problem here regardless, so I'm actually not concerned about that here. Also, that can be dealt with

Also, I'm pretty sure it doesn't break the link for the original cathedral. It would only break the link in the cathedral's entry that links to the replacement, which is... fine. The point of making these hidden from the civilopedia is to avoid their nation entry from showing a ton of buildings that's not actually particularly relevant (they don't do anything particularly noteworthy besides being buildable)

I didn't quite get it

  • In lekmod, Georgia can't buy these buildings for faith. She has to build them.
    As if it makes no sense to let it be built if it can be bought anyway (And it will always be more profitable than building). And we can't turn it off yet, or am I wrong?

  • I don't like it. It seems to contradict the principle of Unciv and Civilopedia.
    Unciv_gFGVr6AM2w

But maybe I would have accepted it if it hadn't been for point 1.

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • In lekmod, Georgia can't buy these buildings for faith. She has to build them.
    As if it makes no sense to let it be built if it can be bought anyway (And it will always be more profitable than building). And we can't turn it off yet, or am I wrong?
  1. So, a bit of a historical note: It used to be able to be bought or built if I remember correctly, and then that was changed at some point. So this isn't that big a deal if that's off here imo and it's far more important that the buildings are buildable
  2. More importantly I'm pretty sure I can switch the uniques around for the beliefs and the buildings so the beliefs don't inherently make the buildings purchasable. Haven't looked into it too deeply yet, but I'm fairly certain the uniques works for this
  • I don't like it. It seems to contradict the principle of Unciv and Civilopedia.

I'd normally agree, but leaving the buildings in the civilopedia clogs up the nation screen for them, which imo is much worse. And the important thing to note there is solely that it's buildable and that it gives production

Copy link
Collaborator

Choose a reason for hiding this comment

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

2. More importantly I'm pretty sure I can switch the uniques around for the beliefs and the buildings so the beliefs don't inherently make the buildings purchasable. Haven't looked into it too deeply yet, but I'm fairly certain the uniques works for this

Ok, double checked this... I need a new unique for this to be implemented, but I'm probably not far off from my original idea

Copy link
Collaborator

Choose a reason for hiding this comment

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

#287

Now you can set the buildings to be unable to be purchased by faith. As far I'm concerned, that should end the discussion there. Whether the buildings are hidden from the civilopedia or not, I do consider it necessary to be replacement unique buildings rather than as a nation unique, and will consider that aspect of this to be a blocker towards merging this PR

DeStup and others added 2 commits February 20, 2025 20:44
Co-authored-by: SeventhM <127357473+SeventhM@users.noreply.github.com>
Co-authored-by: SeventhM <127357473+SeventhM@users.noreply.github.com>
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.

2 participants