[DONE][Suggestion] Prison overview, similar to existing research and production

[Yankes]

Roger that. I'll make a new branch with one commit in it. 

Technically you could can have multiple commits BUT each one need be self contained change, not "Add X" and then 3 times "Fix X" and "Fix again X".
https://www.aleksandrhovhannisyan.com/blog/atomic-git-commits/

Correct PR could have commits like:
"White space cleanups" (100 files changed 0 code touch)
"Refactor Y" (10 files changed, lot of code changed but game logic do not change)
"Add X" (1 file, your feature that change game logic)

[DoxaLogos (JG)]

Technically you could can have multiple commits BUT each one need be self contained change, not "Add X" and then 3 times "Fix X" and "Fix again X".
https://www.aleksandrhovhannisyan.com/blog/atomic-git-commits/

Correct PR could have commits like:
"White space cleanups" (100 files changed 0 code touch)
"Refactor Y" (10 files changed, lot of code changed but game logic do not change)
"Add X" (1 file, your feature that change game logic)

Thanks again for the tips

Just makes it confusing when one repository admin says 1 PR = 1 Commit.  Doesn't leave much room to work with.

I'll see what I can do.

[Meridian]

You didn't do any whitespace cleanups or refactoring.
So it's still one commit.

I don't want to sound like my grandma, but just use common sense.
If you had to maintain this whole madness, would you appreciate if people gave you 10 broken pieces instead of one nice package?
(Have you used 'git blame' before? If yes, you will know how much more difficult it is to search for anything in the git history, if you don't follow these guidelines)

At the end, I would just squash your commits into one anyway, so the repo would be safe... but you wanted to help me, not give me more work.

[DoxaLogos (JG)]

At the end, I would just squash your commits into one anyway, so the repo would be safe... but you wanted to help me, not give me more work.

Will do then...

[DoxaLogos (JG)]

Here's what it looks like with Yanke's group suggestion.

If everyone thinks it looks good enough for merging, let me know. I'll start work on squashing my commits.

[DoxaLogos (JG)]

Alright... second go around.  Here's a PR for this feature request: https://github.com/MeridianOXC/OpenXcom/pull/116

[Meridian]

Substandard quality unfortunately.

1. shows that I have aliens in the Access Lift
2. translations are maintained in Transifex, not in the source code (otherwise they'll just be lost with the next update)
3. missing files for Visual Studio project
4. empty methods (e.g. onOpenTechTreeViewer)
5. unnecessary includes (e.g. ResearchProject)
6. unnecessary data copying (e.g. for(auto itemName)
7. if I understand correctly, it doesn't cover the case when multiple facilities use the same prison type

I'll need to review line by line before merge.

[DoxaLogos (JG)]

I can go back and rework it.  This is why we have a "review" process.

1.  I didn't realize that the access lift had a prison type attached.  EDIT:  After further thought, I just now realize that every facility without a prison type gets a prison type of 0 added to it for backwards compatibility.  In this case, this type 0 is 'aliens'.  I'm not sure how to work around this.  Maybe the capacity(i.e. "aliens")?  Which might still be a problem as they get a capacity of zero.

2.  This is an area I definitely don't know how to use, so I could probably use some pointers.
3. Yep, sorry about that. I build on Linux.  I don't have a windows setup.

4 & 5.  I know why that happened.  I can fix.

6.  I'll have to look at that again.
7.  It doesn't, and I think I know where I missed this.

EDIT: Thanks for the time to provide feedback.  I do appreciate it.

[Meridian]

Merged.

Here's the default translations for modders:

Code: [Select]

  STR_PRISONER_OVERVIEW: "PRISONER OVERVIEW"
  STR_PRISONER: "PRISONER"
  STR_PRISONER_AMOUNT: "AMOUNT"
  STR_PRISONER_INTERROGATED: "INTERROGATED"
  STR_TOTAL_IN_PRISON: "Space Used>{ALT}{0}"
  STR_TOTAL_INTERROGATED: "Interrogated>{ALT}{0}"
  STR_PRISON_TYPE: "Alien Containment"

Additional prison types can be translated like this:

Code: [Select]

  STR_PRISON_TYPE_1: "Aquarium"
  STR_PRISON_TYPE_2: "Prison"
  STR_PRISON_TYPE_3: "Junkyard"


[DoxaLogos (JG)]

Oh wow.  Thanks.  I was going go back and rework things so you didn't have too.