Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#2619 closed task (fixed)

Remove generic 'hele' and 'celt' civs

Reported by: leper Owned by: leper
Priority: Should Have Milestone: Alpha 19
Component: UI & Simulation Keywords:
Cc: Patch:

Description (last modified by leper)

The generic 'hele' and 'celt' civs should be removed and references to them (maps, templates, etc) should be replaced by one of the civs that replaced them (athen/mace/spart, brit/gaul). This also includes a cleanup of the hele/gaul (and replacement civs) templates.

Attachments (1)

HeleAndCeltRemoval.patch (413.5 KB ) - added by Nikos 8 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 by leper, 8 years ago

Keywords: simple added

comment:2 by Nikos, 8 years ago

I might be interested in this to improve my skill in editing the files. Would it include deleting their units and structures as well? Reply in pm or something please, unless this has some form of auto-notification.

Last edited 8 years ago by Nikos (previous) (diff)

in reply to:  2 ; comment:3 by historic_bruno, 8 years ago

Replying to prodigal son:

I might be interested in this to improve my skill in editing the files. Would it include deleting their units and structures as well? Reply in pm or something please, unless this has some form of auto-notification.

Yes, delete the entity templates and civ JSON descriptions, and if you're handy with sed or another tool for searching/replacing text in a large number of files, that would help find references to them.

in reply to:  3 comment:4 by Nikos, 8 years ago

Replying to historic_bruno:

Replying to prodigal son:

I might be interested in this to improve my skill in editing the files. Would it include deleting their units and structures as well? Reply in pm or something please, unless this has some form of auto-notification.

Yes, delete the entity templates and civ JSON descriptions, and if you're handy with sed or another tool for searching/replacing text in a large number of files, that would help find references to them.

Ok you can assign it to me. I'll probably start checking it in a few hours.

Last edited 8 years ago by Nikos (previous) (diff)

by Nikos, 8 years ago

Attachment: HeleAndCeltRemoval.patch added

comment:5 by Nikos, 8 years ago

As an additional note, perhaps some coder could fix the history menu screen so that Hellenes and Celts are now purely cultures, and don't have a civ menu left.

comment:6 by s0600204, 8 years ago

Okay, wow. Good work.

The following are observations from just looking through the patch. Some people might disagree with some of my points, but that's open source for you!

data/mods/public/simulation/ai/common-api/gamestate.js:

  • if (this.playerData.civ == "athen") return "phase_city_athen"; should be retained. Its only the else if ([...]celt that needs removal.

data/mods/public/simulation/ai/petra/headquarters.js:

  • The line var numFortresses [...] should have its indent reduced (by four spaces) to meet coding standards.

data/mods/public/simulation/data/technologies/phase_city.json:

  • {"tech":"phase_city_britons"}, {"tech":"city_phase_gauls"}, may be removed safely from the requirements - the only time they would be true would be if the player chose them from the phase_city_pair_celts pairing, which is celt specific.

data/mods/public/simulation/data/technologies/phase_city_britons.json & data/mods/public/simulation/data/technologies/phase_city_gauls.json:

  • There are no such technologies as phase_city_pair_britons or phase_city_pair_gauls and so attempting to use them as a pair-tech would cause an error. You can safely remove the line entirely.
  • In fact these two files are only used by a player playing as celt when they choose their city-phase tech. This is no longer possible, so these two techs are no longer used in-game and could be marked for deletion from the game files. Similarly, phase_city_pair_celts is a celt civ-specific pair-tech that could also be marked for deletion.

data/mods/public/simulation/data/technologies/phase_town_generic.json:

  • Could we keep the athen specificName, please?

data/mods/public/simulation/helpers/Cheat.js:

  • athen still use their civ-specific phase_town_athen and phase_city_athen tech to reach their phases, so... this requires reverting and just removing the celt specific bits.

data/mods/public/simulation/templates/other/celt_homestead.xml:

  • The prefix of the <ProductionQueue>/<Entities> should be set to {civ}_ instead of gaul_. This means that game will create the equivalent units of the owning civ, rather than potentially giving a brit player gaul units.

data/mods/public/simulation/templates/other/celt_homestead.xml, data/mods/public/simulation/templates/other/celt_hut.xml, data/mods/public/simulation/templates/other/celt_longhouse.xml, data/mods/public/simulation/templates/other/column_doric.xml, data/mods/public/simulation/templates/other/column_doric_fallen.xml, data/mods/public/simulation/templates/other/column_doric_fallen_b.xml, data/mods/public/simulation/templates/other/hellenic_epic_temple.xml, data/mods/public/simulation/templates/other/hellenic_propylaea.xml, data/mods/public/simulation/templates/other/hellenic_royal_stoa.xml, data/mods/public/simulation/templates/other/hellenic_stoa.xml, data/mods/public/simulation/templates/other/plane.xml, data/mods/public/simulation/templates/other/unfinished_greek_temple.xml & data/mods/public/simulation/templates/units/noldor_ship_bireme.xml:

  • Personally, I think the <civ> of these should be set to gaia, but that's just me.

data/mods/public/simulation/templates/other/temp_hele_isp_sword.xml, data/mods/public/simulation/templates/other/temp_hele_super_infantry_p.xml, data/mods/public/simulation/templates/special_units/hele_mechanical_siege_lithobolos_common.xml, data/mods/public/simulation/templates/special_units/hele_mechanical_siege_oxybeles_common.xml, data/mods/public/simulation/templates/units/thebes_sacred_band_hoplitai.xml, data/mods/public/simulation/templates/units/thespian_melanochitones.xml & data/mods/public/simulation/templates/units/thrace_black_cloak.xml:

  • As far as I know, these are no longer in use in-game and can be safely marked for deletion, although I'd hold off on that until someone who can say for definite speaks up.

data/mods/public/simulation/templates/structures/athen_gymnasion.xml & data/mods/public/simulation/templates/structures/spart_syssiton.xml:

  • These two files have the same alteration as issue #2831. I'm guessing you didn't mean to include these changes.

And that's everything that caught my eye. Keep up the good work!

comment:7 by Nikos, 8 years ago

Seems I've missed quite a lot, not surprisingly though, most of those are things I wouldn't understand by myself.

Some of the units you suggest to remove might be useful for scenarios and a few could even get to be trainable at some point.

On last part, I know, but had already submitted when I noticed and thought it's a very minor thing anyway.

Thanks for the detailed check:) I'll probably let the fixes to someone else though, still no clue on several of them.

comment:8 by Stan‘, 8 years ago

Make sure you adapt the scenarios Xml's too. If the unit doesn't exist it will crash atlas.

in reply to:  8 comment:9 by Nikos, 8 years ago

Replying to stanislas69:

Make sure you adapt the scenarios Xml's too. If the unit doesn't exist it will crash atlas.

I checked those. Didn't test them one by one but I removed/replaced the references and units needed

comment:10 by Itms, 8 years ago

In 16006:

Add wall style for Seleucids in the random map scripts. Currently, Hellenic walls are used. Fixes a bug in the Fortress random map.
Patch by FeXoR.

Fixes #2942, refs #2619.

comment:11 by Itms, 8 years ago

In 16174:

Implement the Rotary Mill aura for Celtic factions. Patch by niektb, fixes #2900.

Also remove the placeholder technology and the rotary mill generic Celtic template, and rename the rotary mill portrait. Refs #2619.

comment:12 by historic_bruno, 8 years ago

It's kinda weird that the only place in the UI with "grouping" for Celts and Hellenes is the civilization info dialog, everywhere else is just a sorted list (see game setup, struct tree, and Atlas). We also have to keep a generic civ template specially for that window that serves no other purpose.

I think that list should be made consistent with all the others by removing the grouping. Unless we could have dropdowns with unselectable entries or used some other convention to sort them into groups, but really it seems unnecessary.

comment:13 by s0600204, 8 years ago

As the person who proposed the current grouping of civs in the civinfo dialog back in #1734... I agree. Reverting the changes made to gui/civinfo/civinfo.js in r12905 should do it.

Also, loadCivData() has since been modified so that if true is passed as its first (and only) argument, the function will only return civs selectable/playable ingame. This could be used here.

comment:14 by leper, 8 years ago

Description: modified (diff)

Agreed.

comment:15 by leper, 8 years ago

In 16415:

Remove grouping from civ info screen. Only show playable civs. Refs #2619.

Does not use loadCivData(true) as the civ info screen can also be opened from the
gamesetup screen and both use g_CivData.

comment:16 by leper, 8 years ago

Owner: set to leper
Resolution: fixed
Status: newclosed

In 16416:

Removes generic hele and celt civs. Based on patch by prodigalson and
comments by s0600204. Fixes #2619.

Also removes the theb civ json file.

comment:17 by leper, 8 years ago

Keywords: simple removed
Milestone: BacklogAlpha 19

Thanks for the patch and the comment.

Note: See TracTickets for help on using tickets.