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

Draft: Remove unused code related to csv #717

Closed
wants to merge 1 commit into from

Conversation

royfalk
Copy link
Contributor

@royfalk royfalk commented Jul 22, 2022

The file VegaPersonalities.csv isn't used.

@ministerofinformation / @Loki1950 - please let me know if you think this file should have existed and used recently.

Code Changes:

Issues:

Purpose:

  • What is this pull request trying to do? Remove CSV code by either converting to JSON or deleting if not used.

The file VegaPersonalities isn't used.
Copy link
Member

@Loki1950 Loki1950 left a comment

Choose a reason for hiding this comment

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

not sure about the personality file but it may be related to the story line

@@ -173,7 +148,7 @@ static AIEvents::ElemAttrMap *getLogicOrInterrupt(string name,
vsUMap<string, AIEvents::ElemAttrMap *> &mymap,
int personalityseed) {
string append = "agg";
static vsUMap<string, string> myappend = getAITypes();
static vsUMap<string, string> myappend;
Copy link
Member

Choose a reason for hiding this comment

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

how will this get populated now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you look at line 113-115, this is what's happening anyway. The file 'VegaPersonalities' doesn't exist in either VS or WCPU. I've trimmed the code but nothing's changed. At least, that was the intention.
I specifically asked @Loki1950, as he's our resident historian for such things.
As I can't figure out from the code what they wanted to do and this appears to be a stub for some future feature, I vote to delete it. If/When we get to developing, we'll hopefully document it as well :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I just checked. The file 'VegaPersonalities.csv' does, in fact, exist in PWCU. Three copies of it. One under the 'ai' folder, another under the 'ai.easy' folder, and the third under the 'ai.hard' folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huh. I actually checked in what I assumed was the repository. Is it something you're running on your computer or is it in the repo as well? If it is, can you point me to it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@stephengtuggy stephengtuggy left a comment

Choose a reason for hiding this comment

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

I would rather not make this change at this time. This seems like one of those "fences" that we don't fully understand the purpose of yet. Once we do, we can come back and revisit.

@royfalk
Copy link
Contributor Author

royfalk commented Jul 24, 2022

I absolutely don't understand the purpose of this. However, it also isn't active yet and never will be. The only two ways to figure this out are to ask one of the original devs and hope he remembers after a decade or meditate on the entire AI code and hope it becomes clearer then.
As noted above, this code doesn't do anything if the file is missing.

@Loki1950
Copy link
Member

Loki1950 commented Aug 3, 2022

I have a possible reason for it's existence as @stephengtuggy has mentioned it holds the AI profiles of NPC's in the story line since Upon a the Coldest Sea is just a stub ATM that file is not needed yet as it is in Privateer which does have a full story.

@royfalk
Copy link
Contributor Author

royfalk commented Aug 3, 2022

Based on the example in the link above, I'll start working on converting it instead of simply deleting the whole function.

@royfalk royfalk changed the title Remove unused code related to csv Draft: Remove unused code related to csv Aug 3, 2022
@royfalk
Copy link
Contributor Author

royfalk commented Aug 23, 2022

OK. So I looked at the PU data and tried to make it work for VS and it crashed nicely. Looks like I'm going to have to run PU and debug that.

@royfalk
Copy link
Contributor Author

royfalk commented Sep 5, 2022

I'm going to close this. The code is needed and I may as well start from scratch. There's nothing here to salvage.

@royfalk royfalk closed this Sep 5, 2022
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.

4 participants