Reworked Research Code

[hellrazor]

@Meridian

You mentioned some performance optimizations, for checking the lists differently.

Could you explain to me how these would/could work? I really would appreciate the knowledge, thanks in advance.

Since you avoided my direct question, I assume the answer is "no, you will continue working on it if I show you more issues".

Sorry that got lost on the way.
If i may, I will try to answer your question now.
You already mentioned the performance. I can not really judge how performant in regards to CPU cycles the code is.
I am honest i do not know.

If so, I can respond only with:
I regret that I told you about the issue, which you are trying to solve here... I should have kept my mouth shut.
This whole discussion is my fault, I just wish I could un-tell you about it....

Looks like i missunderstood you and acted or replied in a way which may have seen unappropriate for you.

I won't tell you about the other issues, because I don't want to continue along this path.
This is my last post on this topic.

If thats your decision. Ok understandable.
Even thou someone with more programming experience telling me about issues, would be really good.

I understand you just want to help... but starting with one of the most perplexing pieces of oxc code with lots of side effects is just a bad idea.

In regard of the overall game fps performance?
Or about that it would break the research in general?
I do not know, but I recall in regard of research in OpenXcom, this Thread here

Were you actually showed the core issue, whats wrong with the research code.
Even thou "requires" were actually never the issue.
The problem is that alive aliens, which are not leaders or commanders, only are available for research as long as on their "getOneFree" list contains entries, which do not show up as discovered and as long as their lookup is not satiesfied.
Meaning the amount of times they can be researched is limited.
When you add a list of unlocks to a specfic alien rank for example all alien engineers, which is longer then the entries of the getOneFree list then you could get stuck with the complete game research.

I fixed that. Mainly out of self interest because it breaks the research tree of my mod depending how the player interrogates alive aliens to progress in the research tree.
Maybe this is also a possible problem with players sometimes getting stuck on research trees of other mods to.

Good luck, whatever your decision to continue may be...

Danke

[hellrazor]

Diff to original:

Code: [Select]

1151a1152
>
1156,1160c1157
< std::vector<const RuleResearch *>::const_iterator itDiscovered = std::find(discovered.begin(), discovered.end(), research);
<
< bool liveAlien = mod->getUnit(research->getName()) != 0;
<
< if (itDiscovered != discovered.end())
---
> if (!research->getRequirements().empty())
1162,1175c1159
< bool cull = true;
< if (!research->getGetOneFree().empty())
< {
< for (std::vector<std::string>::const_iterator ohBoy = research->getGetOneFree().begin(); ohBoy != research->getGetOneFree().end(); ++ohBoy)
< {
< std::vector<const RuleResearch *>::const_iterator more_iteration = std::find(discovered.begin(), discovered.end(), mod->getResearch(*ohBoy));
< if (more_iteration == discovered.end())
< {
< cull = false;
< break;
< }
< }
< }
< if (!liveAlien && cull)
---
>                         if(!isResearched(research->getRequirements()))
1179,1203d1162
< else
< {
< std::vector<std::string>::const_iterator leaderCheck = std::find(research->getUnlocked().begin(), research->getUnlocked().end(), "STR_LEADER_PLUS");
< std::vector<std::string>::const_iterator cmnderCheck = std::find(research->getUnlocked().begin(), research->getUnlocked().end(), "STR_COMMANDER_PLUS");
<
< bool leader ( leaderCheck != research->getUnlocked().end());
< bool cmnder ( cmnderCheck != research->getUnlocked().end());
<
< if (leader)
< {
< std::vector<const RuleResearch*>::const_iterator found = std::find(discovered.begin(), discovered.end(), mod->getResearch("STR_LEADER_PLUS"));
< if (found == discovered.end())
< cull = false;
< }
<
< if (cmnder)
< {
< std::vector<const RuleResearch*>::const_iterator found = std::find(discovered.begin(), discovered.end(), mod->getResearch("STR_COMMANDER_PLUS"));
< if (found == discovered.end())
< cull = false;
< }
<
< if (cull)
< continue;
< }
1205c1164,1186
<
---
>                 std::vector<const RuleResearch *>::const_iterator itDiscovered = std::find(discovered.begin(), discovered.end(), research);
>                 if(itDiscovered != discovered.end())
>                 {
>                         bool removeFromMenu = true;
>                         if(!research->getGetOneFree().empty())
>                         {
>                                 if(!isResearched(research->getGetOneFree()))
>                                 {
>                                         removeFromMenu = false;
>                                 }
>                         }
>                         if(research->getCost() != 0 && !research->getUnlocked().empty())
>                         {
>                                 if(!isResearched(research->getUnlocked()))
>                                 {
>                                         removeFromMenu = false;
>                                 }
>                         }
>                         if(removeFromMenu)
>                         {
>                                 continue;
>                         }
>                 }
1214,1227d1194
< if (!research->getRequirements().empty())
< {
< size_t tally(0);
< for (size_t itreq = 0; itreq != research->getRequirements().size(); ++itreq)
< {
< itDiscovered = std::find(discovered.begin(), discovered.end(), mod->getResearch(research->getRequirements().at(itreq)));
< if (itDiscovered != discovered.end())
< {
< tally++;
< }
< }
< if (tally != research->getRequirements().size())
< continue;
< }
1273,1275d1239
< std::vector<std::string> deps = r->getDependencies();
< const std::vector<const RuleResearch *> & discovered(getDiscoveredResearch());
< bool liveAlien = mod->getUnit(r->getName()) != 0;
1280,1305c1244
< else if (liveAlien)
< {
< if (!r->getGetOneFree().empty())
< {
< std::vector<std::string>::const_iterator leaderCheck = std::find(r->getUnlocked().begin(), r->getUnlocked().end(), "STR_LEADER_PLUS");
< std::vector<std::string>::const_iterator cmnderCheck = std::find(r->getUnlocked().begin(), r->getUnlocked().end(), "STR_COMMANDER_PLUS");
<
< bool leader ( leaderCheck != r->getUnlocked().end());
< bool cmnder ( cmnderCheck != r->getUnlocked().end());
<
< if (leader)
< {
< std::vector<const RuleResearch*>::const_iterator found = std::find(discovered.begin(), discovered.end(), mod->getResearch("STR_LEADER_PLUS"));
< if (found == discovered.end())
< return true;
< }
<
< if (cmnder)
< {
< std::vector<const RuleResearch*>::const_iterator found = std::find(discovered.begin(), discovered.end(), mod->getResearch("STR_COMMANDER_PLUS"));
< if (found == discovered.end())
< return true;
< }
< }
< }
< for (std::vector<std::string>::const_iterator itFree = r->getGetOneFree().begin(); itFree != r->getGetOneFree().end(); ++itFree)
---
> if (!r->getDependencies().empty())
1307,1317c1246
< if (std::find(unlocked.begin(), unlocked.end(), mod->getResearch(*itFree)) == unlocked.end())
< {
< return true;
< }
< }
<
< for (std::vector<std::string>::const_iterator iter = deps.begin(); iter != deps.end(); ++ iter)
< {
< RuleResearch *research = mod->getResearch(*iter);
< std::vector<const RuleResearch *>::const_iterator itDiscovered = std::find(discovered.begin(), discovered.end(), research);
< if (itDiscovered == discovered.end())
---
>                 if(!isResearched(r->getDependencies()))
1322c1251,1270
<
---
>         bool allGetOneFree = false;
>         bool allUnlocks = false;
>         if(!r->getGetOneFree().empty())
>         {
>                 if(isResearched(r->getGetOneFree()))
> {
> allGetOneFree = true;
> }
>         }
>         if(!r->getUnlocked().empty())
>         {
>                 if(isResearched(r->getUnlocked()))
>                 {
>                         allUnlocks = true;
>                 }
>         }
>         if(allGetOneFree && allUnlocks)
>         {
>                 return false;
>         }
Tested against vanilla xcom1 and Hardmode Expansion more tests welcome.
Diff To Original

EDIT: Many thanks to Meridian for pointing out the issues with performance and the alive aliens. Fixed now.

[hellrazor]

Pull request diff https://github.com/SupSuper/OpenXcom/pull/1153/commits/c8decabd83827328fd2b085aca40ca1ffb319c5d

Pull request: https://github.com/SupSuper/OpenXcom/pull/1153

[hellrazor]

Closed my PR: https://github.com/SupSuper/OpenXcom/pull/1153

Meridian found some more bugs and fixed them, see https://openxcom.org/forum/index.php/topic,5494.0.html

He made another Pull request, which already contains most of my code, with some slight changes and a lot of other bug fixes I was not aware of.

See Meridians Pull Request here: https://github.com/SupSuper/OpenXcom/pull/1155

Thanks Meridian!