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

3x replacement factions #5252

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

Conversation

joonicks
Copy link
Contributor

@joonicks joonicks commented Sep 1, 2021

replacing autogenerated factions with slightly more fleshed out ones

@impaktor
Copy link
Member

impaktor commented Sep 2, 2021

It was decided in the dev team, some time ago, that all factions but the 4 first will be removed, (e.g. #3629).

@Gliese852
Copy link
Contributor

@impaktor I do not see a collective decision to delete factions in #3629

@impaktor
Copy link
Member

impaktor commented Sep 2, 2021

@Gliese852 Indeed, it's taken for given there, I don't remember who/where the discussion was had, and if anyone is wondering, I don't think I was the driving force behind it, although I agree with it. Spamming the universe with +100 factions, that differ only in "a name" and "a color" (sure, there's the illegal goods list), is not meaningful, compared to a handful of hand scripted, that truly give different "feeling" when being there. Just like in Frontier: being in Imperial vs. Alliance vs. Federation space did feel different, the stations, the adverts, the military, the ships on sale, the market prices, etc.

@Gliese852
Copy link
Contributor

@impaktor yes, I understand your position, but I think that space is huge, and there must be a lot of things. And I'm sure there must be room for any number of factions if contributors want to describe them. Personally, I like to explore space and see all these homeworlds of factions, it seems that there is a lot of life.

@joonicks joonicks changed the title 2x replacement factions 3x replacement factions Sep 2, 2021
@joonicks
Copy link
Contributor Author

joonicks commented Sep 2, 2021

I just suggested to Gliese in chat a quest to visit all the faction homeworlds.

Maybe its best to gradually replace the autogenerated factions over time and also reduce the number to ~50-80 or so, with a handful of large dominating factions and the rest being smaller less expansive.

If my lore ideas gets added, factions would gain a bunch more substance and become more tangible.

by request (gliese)

2x replacement factions

replacing autogenerated factions with slightly more fleshed out ones
@zonkmachine
Copy link
Member

@joonicks I could cherry-pick this PR on top of master after removing the conflicting files with git rm.
The illegal goods names was changed in c4109e0 so the ones in all caps gives a warning.

Lua Warning: Faction[Julian science community]: Invalid commodity name LIQUOR passed to IllegalGoodsProbability.

@joonicks
Copy link
Contributor Author

joonicks commented Sep 4, 2021

I messed up and based my changes on an old git and now I dont know how to fix it proper

by request (gliese)

2x replacement factions

replacing autogenerated factions with slightly more fleshed out ones
Copy link
Member

@Web-eWorks Web-eWorks left a comment

Choose a reason for hiding this comment

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

There are several places where the strings in this PR need some sprucing up; I'll leave it to @impaktor / @nozmajner to determine if the short lore proposed for these factions is compliant with the overall lore of the game.

I'm not opposed to merging this PR as it replaces three otherwise blank factions, but the review comments will need to be addressed before merge.

data/factions/047_Druello_pirates.lua Outdated Show resolved Hide resolved
data/factions/047_Druello_pirates.lua Outdated Show resolved Hide resolved
data/factions/077_Tolan_Kingdom.lua Outdated Show resolved Hide resolved
data/factions/077_Tolan_Kingdom.lua Outdated Show resolved Hide resolved
@impaktor impaktor force-pushed the master branch 3 times, most recently from 876b6fc to b5bc47c Compare July 6, 2022 13:11
@impaktor impaktor force-pushed the master branch 2 times, most recently from 90e4530 to 6a3a044 Compare January 26, 2023 21:29
@zonkmachine
Copy link
Member

It looks like all changes suggested in review have been done. If the English is acceptable I think this should be merged.

Copy link
Member

@Web-eWorks Web-eWorks left a comment

Choose a reason for hiding this comment

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

These look fine to merge to me at this time, if needed we can make corrections/improvements once in master. I'm generally in favor of improving the auto-generated factions, and this is a good step in the right direction.

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.

None yet

5 participants