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

Reorganize FOCS files #1718

Merged
merged 21 commits into from Sep 14, 2017

Conversation

dbenage-cx
Copy link
Member

Reorganizes FOCS files in a consistent manner using sub directories and limiting to one definition per file.

Superceeds #1631

Open PRs with conflicts: #1709 #1343

@dbenage-cx dbenage-cx added category:refactoring The Issue/PR describes or contains an improved implementation. component:content scripting The Issue/PR deals with the FOCS language, turn events or the universe generator. labels Sep 2, 2017
@dbenage-cx dbenage-cx added this to the v0.4.8 milestone Sep 2, 2017
@o01eg
Copy link
Contributor

o01eg commented Sep 2, 2017

Could you also move FOCS read only by the server or the AIs move to separate directory like default/server/? As example there are scripts for empire statistic.

@dbenage-cx
Copy link
Member Author

@o01eg I fail to see what would establish statistics to the domain of server? I also do not see any definitions (or other files within default/scripting) that are specific to the AI.

@o01eg
Copy link
Contributor

o01eg commented Sep 3, 2017

I fail to see what would establish statistics to the domain of server?

As I've just rechecked only server calls code Universe::UpdateStatRecords to parse statistics scripts.

I also do not see any definitions (or other files within default/scripting) that are specific to the AI.

Ok.

@geoffthemedio
Copy link
Member

I don't think categorizing stuff as "server only" is useful conceptually. AI stuff is doing something conceptually different. Statistics, even if only actually parsed by the server, are just another part of the game content, and they do appear in the client.

@LGM-Doyle
Copy link
Contributor

Thanks for the notice about the conflict with #1343.

@o01eg
Copy link
Contributor

o01eg commented Sep 4, 2017

@geoffthemedio It's about changes could be made without breaking compatibility with the client.

@geoffthemedio
Copy link
Member

I think the client would require matching stringtable updates for changes to statistics, so that could be potentially misleading...

@MatGB
Copy link
Member

MatGB commented Sep 4, 2017

Unlike last time this doesn't conflict with any of my current work so that's a relief.

I would rather keep the stats where they are, they're written using FOCS and I sometimes use them for testing purposes, plus at some point we want to gate them in some way based on intelligence/spying/diplomacy which'll be part of scripting.

But it's a soft preference, if there's a strong reason to change I don't object, I'm just not sure I understand why you'd want to.

Regarding the PR overall, it looks fine to me, haven't got time to test it currently but the principle is sound.

@dbenage-cx
Copy link
Member Author

@o01eg Thank you for the info on statistics, I had not looked into those in the past.

@dbenage-cx dbenage-cx merged commit e08cbe8 into freeorion:master Sep 14, 2017
@dbenage-cx dbenage-cx added the status:merged All relevant commits of this PR were merged into the master development branch. label Sep 14, 2017
@dbenage-cx dbenage-cx deleted the ref-1631_focs-single-def-file branch September 14, 2017 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:refactoring The Issue/PR describes or contains an improved implementation. component:content scripting The Issue/PR deals with the FOCS language, turn events or the universe generator. status:merged All relevant commits of this PR were merged into the master development branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants