yairm210 / Unciv

Open-source Android/Desktop remake of Civ V
Mozilla Public License 2.0
8.39k stars 1.57k forks source link

Resources in tech tree locked #10317

Closed hackedpassword closed 11 months ago

hackedpassword commented 11 months ago

Trying to relocate strategic resources to other techs - but they won't move from the default G&K positions. I'll break it down:

v4.8.13 Mod: Z2 baseruleset: false (is expansion)

We'll use coal as the target resource example. I'm moving the revealedBy tech to Steam Power. Should be straight forward, change the revealedBy. In-game, the tech tree is unchanged.

I let this go for several iterations as I continued working in the tech tree (WIP). No change. Updated to 4.8.13, no change (though that did fix a missing resource icons but). Tried an exclusion of the parent ruleset config via ModOptions, still nothing.

Screenshot_20231014_133312

In this instance, the behavior appears that the parent ruleset G&K is taking precedence. Did I not configure something to allow moving the resource? Searching the G&K repo dir reveals no additional config needed.

SomeTroglodyte commented 11 months ago

I wonder why does the Civilopedia page for resources not list the revealed by? Did we not code that?? No - even in #4628 no trace... To-do.

image Huh ???? image Fourth entry as read from file - is OK ???????? image After conversion to a Map - is reverted ????????????

Oh my. That seems like an unsolvable conundrum, right? The programming language itself is mocking us, right?

SPOILERS There's more than one entry named "Coal" in your json - the last one in always wins, unlike F***

Your techsToRemove should be no-ops as you're adding them back right after - unless the ruleset merge doesn't overwrite.

SeventhM commented 11 months ago

Ah, Yeah, remove your copy paste versions of the resources. Things typically override in order of base, then mod, top to bottom. If you list an object twice, it usually only takes the last one (Rekmod actually used to have a whole civ overridden by accident this way). I think you forgot that you can set G&K as a base ruleset in the mod checker, making the duplicates unnecessary

hackedpassword commented 11 months ago

Wow, how did this happen? Dupe resource? Did I just work on this too late? (likely)

Before I go any further, I appreciate you guys taking the time to investigate. Thought I covered everything... wrong!!!

The programming language itself is mocking us, right?

Absolutely. Well just me this time. Ugh!

you can set G&K as a base ruleset in the mod checker, making the duplicates unnecessary

Really. Having to specify the dupes seemed to be kinda ridiculous. I'll definitely check that out. Nope, didn't know. :)

SomeTroglodyte commented 11 months ago

Actually, our validation can't finger this kind of mistake, because it runs after the loader, which already hides dupes.

@SeventhM - this would log the problem to the console and not load Z2 at all:

// Ruleset.kt, Line 87, just before createHashmap returns:
        if (hashMap.size != items.size) {
            val dupes = items.groupBy { it.name }.filter { it.value.size > 1 }.keys
            throw kotlin.IllegalStateException("Duplicate items in some json: $dupes")
        }

... no idea yet how to get such info through to the mod check display in Options - except a lot of extra fields and methods to store loading problems into the Ruleset to retrieve later - any better inspiration?

hackedpassword commented 11 months ago

Maybe should not have closed?

Hey, ok, I'm combing the wiki, stepping through unciv-mod-expansions, only instance of specifying a baseruleset mod is for incompatibility. Trog, your mod is the closest I have found to specifying a mod, but inversely.

set G&K as a base ruleset in the mod checker

Screenshot_20231014_211010

We talking about that, or a config option? I'm not trying to sound clueless, just hitting a lot of dead ends.

SeventhM commented 11 months ago

We talking about that

Yeah. That should combine your mod with the base when checking for mod errors, meaning you shouldn't need to redefine stuff

SomeTroglodyte commented 11 months ago

Well, Aliens is years outdated. There is some code lying around never finished... I wanted the Aliens starting location working nuder random generation such that they would have their spaceship near... That old code was before MapRegions were introduced.

specifying a baseruleset

That's the thing - historically, Unciv didn't care at all about 'sense' in combining mods. Still a WIP. Later, Unciv still only does some sense checking when you tell it to - in new game and options-modcheck.

"Check based on" is just the way to do half of what newgame does - specify a combination. An extension mod otherwise is allowed to be incomplete - refer to stuff it doesn't define itself. So, you can check it standalone and expect problems (and the checker actually has a reduced mode for that), or in combination, and that is what that selectbox does... The whole page isn't optimal, I'm always tempted to do a better one, but so far had no clear inspiration how it should work. The checking extensions against 'none' shouldn't be the default, even maybe not be offered at all. Someday it will use the compatibility uniques, maybe that will spark a clear concept.

Some first doc on how objects are treated by base vs extension mods would be under "There are three main kinds of mods" in Introduction, and going by that, replacing elements isn't supported at all. Oh my, down there a dyslexic was at work... Why is mistaking 'their' and 'there', 'poring' and 'pouring', or 'affect' and 'effect' so prevalent? How can the authors stand the ensuing physical pain?

Back to Z2 - I wondered right away, the theme sounds more like a base mod would be more adequate. But hey, maybe that's interim so you get unmodded base stuff to play with while the themed additions are still few, and will change later. So the combining happens in new game, mandatory, so you only need to specify what is new, changed, or to remove. And removal, unlice modding Civ5, is far away from the place where adding happens.

SomeTroglodyte commented 11 months ago

As for that createHashmap patch up there, the text would actually be displayed to the user, as separate error when they click 'reload mods' on the mod checker. But - no, don't like.

hackedpassword commented 11 months ago

Z2 has become somewhat of a proving grounds for modding unciv. I have a clear direction and outcome, but there's a lot of fun things I've picked up along the way that didn't have anything to do with the intended mod dev. Like the tech tree, something tiny needed to be changed, led to a whole tree shuffle. And it was fun as heck remaking it!

You're right, there is reason to make Z2 a base ruleset. Because the direction is 100% to capture the Hyrule map only as an Unciv battleground, to simplify my learning curve I stripped all the map generation params out, making it unsuitable as a base. Maybe eventually I'll open up to generating other places in the Zelda world - that's a whole project of its own.

The more immediate plan is to split off the content/assets that are not specific to intended Z2 gameplay as their own mod(s) once Z2 is ready in full. The new ordnance and advanced GDR's are super great for example though outside the theme scope. Lot of really good things going on on this mod - bridges, era-based villagers from the game, even lava and caves - and still I see so much more potential. Lots more to learn too.

Has anyone forked unciv as a game engine to become a different game entirely, to your knowledge?

hackedpassword commented 11 months ago

@SeventhM fyi after analyzing all the garbage dupes in that resources json, what I found is within the github web interface via chrome, copy-paste, specifically copy, gets glitchy when scrolling. The repeated dupe fragments made no sense. The behavior looks like fluid responsiveness is where that glitchiness lies. Will have to be more careful.

SeventhM commented 11 months ago

except a lot of extra fields and methods to store loading problems into the Ruleset to retrieve later

Maybe you push down the amount of fields to just a couple

// in Ruleset.kt
val log = ArrayList<String>()

private fun <T : INamed> createHashmap(items: Array<T>): LinkedHashMap<String, T> {
        val hashMap = LinkedHashMap<String, T>(items.size)
        for (item in items) {
            if(hashMap[item.name] == null)
            {
                hashMap[item.name] = item
                (item as? IRulesetObject)?.originRuleset = name
            }
            else
            {
                log.add(item.name)
            }
        }
        return hashMap
    }
SomeTroglodyte commented 11 months ago

Yeah, and what do you do with that collection to make it useful? Need to pass it somehow to RulesetValidator - either by making it a permanent field of the Ruleset or by returning it from the load method and have the caller somehow pass it through (nah). And to allow RulesetValidator to make something readable out of it, you'll need to keep more info, which is why I used the plural...

And please don't set your IDE to format C style - https://kotlinlang.org/docs/coding-conventions.html#control-flow-statements

SeventhM commented 11 months ago

making it a permanent field of the Ruleset

That's what this code snippet does

set your IDE to format C style

I didn't. Habits on my part