search

yairm210 / Civ-IV

A Civ IV mod for Unciv
27 stars 18 forks source link

Update #36

Closed RobLoach closed 1 month ago

RobLoach commented 1 month ago

Been aiming to fill in some of the missing pieces on this mod. Am opening this as a draft to get some early feedback. If there's any suggestions you could make along the way, I'm all ears.

yairm210 commented 1 month ago

Damn, it looks like you've put a lot more effort into this than I have been! What I can see looks good, I'll just make you an admin on this repo and as far as I'm concerned you have free reign to change whatever you see fit :)

RobLoach commented 1 month ago

Thanks for the add. Once I fix up the icons a bit, and log the todos, I'll push this up. It's looking pretty solid though.

Do you know if Religion is supported yet? I've added all the religion things (Great Prophets, Temples, Cathedrals, etc), but I'm unsure how to actually Found a religion. Will likely keep it disabled for now.

yairm210 commented 1 month ago

Religion is supported in Unciv, but it's Civ V flavored religion - I don't recall what religion was like in Civ IV, haven't played it in over a decade

RobLoach commented 1 month ago

First Civ to discover a certain technology founds the designated Religion.

For example, when you discover Monotheism, and you are the first to do so, you automatically get the Jewish religion.

There isn't a concept of Faith. It just happens.

RobLoach commented 1 month ago

@yairm210 I believe the majority of Civ IV is in there now. What remains is tagged in TODO notes, which I can move to the issue queue :+1:

Before I squash-merge, I've got a couple questions...

  1. Rename: Could we rename this repository to Civ IV to match the naming convention of how Civ V is presented? I don't have the perms to rename it. Would be nice to have a clean name there.

    Screenshot from 2024-10-10 23-08-21

  2. Expansions: I've split some of it to expansion mods Warlords and Beyond the Sword repos, and <if [modFilter] is enabled> worked marvelously, thanks. Think it's worth having a <if [modFilter] is disabled> ?

  3. Religion: I got around it by having the Religions as Pantheons. While it worked well, I've found that Missionaries couldn't actually spread the Pantheon with Can Spread Religion. I guess this is intentional, do we need a Can Spread Pantheon? source

  4. Conditional Logic: Civil Service required Mathematics AND (Code of Laws OR Feudalism). Is there a way to implement or conditions? Has there been discussion around more conditional logic?

    Only available <after discovering [Code of Laws]> <or> <after discovering [Feudalism]>

Thanks again! This is really solid. If you'd rather I take on the rename and such, you could give me full Admin/Owner rights, then I could tag it as unciv-mod-rulesets too.

yairm210 commented 1 month ago

Regarding rename: I wanted the repo to be clear that this is an Unciv mod, and not some official/unofficial Civ IV code or mod - maybe we can have an "internal mod name that differs from repo name"? I'll need to think about whether this is a good idea Regarding "when modFilter is disables" - sounds good, I'll add that Regarding conditional logic - this is one of those long-standing issues that we've discussed many many times. Latest iteration is here

Yes, in Civ V pantheons cannot be spread. In Civ IV they can? If so they we'll need a new unique

RobLoach commented 1 month ago

Regarding rename: I wanted the repo to be clear that this is an Unciv mod, and not some official/unofficial Civ IV code or mod

I believe having the repo named Civ IV won't confuse others into thinking it's the official Civ IV code. They'll see the description and see reference to Unciv :wink: ... Also unsure where we would get that human readable name through the GitHub API too. Made an issue to track that over at https://github.com/yairm210/Unciv-IV-mod/issues/38

Regarding "when modFilter is disables" - sounds good, I'll add that

This would be helpful to disable some features that were replaced in BTS. If I find where it applies here again, I'll make an Draft PR to allow testing.

Regarding conditional logic - this is one of those long-standing issues that we've discussed many many times

Most excellent, subscribed. Sounds like a tricky one.

Yes, in Civ V pantheons cannot be spread. In Civ IV they can? If so they we'll need a new unique

That makes sense, thanks. I've split off an issue over at https://github.com/yairm210/Unciv-IV-mod/issues/39 to track that discussion.

In the mean time, this is looking solid. I'll add the remaining icons, see if I can get the nation dialog text, do another round of testing, and then merge this up.