OpenXcom Forum
Modding => OpenXcom Extended => OXCE Suggestions DONE => Topic started by: aku on November 30, 2022, 04:07:07 pm
-
I just spent some time looking through all the prison facilities in my bases for that one specific prisoner I forgot where I put and I thought it would be handy to have a geoscape hotkey to display an overview of all prison facilities in all the bases, similar to already existing research and production overviews. And clicking on base name on any prisoner under it would take you directly to that specific prison so you can ship the prisoner where you need it.
Not sure how to handle different prisoner types in piratez like beasts, creatures, robots, trained auxilliares like dogs and werewolves and stuff. I guess the simplest approach would be to put them all in there and separate by facility, so base 1: prison, beast den, cryo prison, base 2: large prison, beast den... and so on.
Doable?
-
todolisted, but with low priority
-
Since I did the original impetus of the Global Research overview that got merged, I can tackle this. Looks like low hanging fruit and easy to do while Meridian and Yankes work on higher priority features.
-
So, I almost have this feature implemented, but I need feedback. My biggest issue is tallying up the space you have for containment for all prison types. This includes the prison type for storage that holds things like robots. I'm unsure how to exclude them because prison is a prison no matter what type of being you are.
Below is the screenshot of my nearly complete work. Notice the total available space is massive even though I only have a prison cell and animal pens (X-COM files). The rest would be related to my storage facilities (prison type 4 in Xcom files). Is it okay to include them in the total? Should I even worry about displaying overall totals, and just only display currently listed captives in containment?
-
So, I almost have this feature implemented, but I need feedback. My biggest issue is tallying up the space you have for containment for all prison types. This includes the prison type for storage that holds things like robots. I'm unsure how to exclude them because prison is a prison no matter what type of being you are.
Below is the screenshot of my nearly complete work. Notice the total available space is massive even though I only have a prison cell and animal pens (X-COM files). The rest would be related to my storage facilities (prison type 4 in Xcom files). Is it okay to include them in the total? Should I even worry about displaying overall totals, and just only display currently listed captives in containment?
Impressive work!
What if you consider type four items apart from the others? (It is related to robots, vehicles, and similar if I remember)
However if you finally don't include type 4 prisoners in the list I would not consider type 4 space in your feature.
Enviado desde mi Mi A2 Lite mediante Tapatalk
-
Good question. I think the only problem I have with excluding a prison type fixed to #4 in this case is that I don't know how future modders would use it. Just because it's robot/mechanical in one mod doesn't necessarily meant it will be in another mod (unless I misunderstand something).
I'm tempted just to remove the total capacity from the window only, and I think that will the one problem(i.e. big number). However, you would still see the type 4 prisoners in the list if you had them.
-
all prisoners are equal, the engine does not and should not differentiate between them, and definitely not hardcoded
if the modders found some unintended side effect again -- I honestly don't know what that type 4 is -- then it's not my problem to solve
-
all prisoners are equal, the engine does not and should not differentiate between them, and definitely not hardcoded
if the modders found some unintended side effect again -- I honestly don't know what that type 4 is -- then it's not my problem to solve
I would agree.
I was just wonder if there was a better way to display the totals without looking so surprising when someone sees 156 available space while thinking they only had 20 spaces from a prison cell and an animal pen (or alien containment). This is why I'm inclined to drop the available containment space stat at least and just show who's currently captive and being studied or interrogated.
EDIT:
I'm leaning toward displaying the prison type next to the prisoner on the list instead.
-
Here it is with total available capacity removed.
-
all prisoners are equal, the engine does not and should not differentiate between them, and definitely not hardcoded
if the modders found some unintended side effect again -- I honestly don't know what that type 4 is -- then it's not my problem to solve
Type 4 are robotic enemies, like Cyberdiscs or Sectopods, but mostly regular robotic enemies (human made automated drones and the like).
They do not use any special rules or side effects. Their only notable feature is that this prison type is used on standard warehouses - contrary to some rumours, we don't put evil roombas in cages. :)
-
PR is up if you ever want to merge :)
https://github.com/MeridianOXC/OpenXcom/pull/115
I added the prison where the prisoner is being held captive to the list as well. See below.
-
Please 1 PR = 1 commit
10 commits with random descriptions is really not acceptable.
Also it says WIP, please PR only after it is considered finished.
As for adding prison type, I don't think it will fit all prisoner names and prison names with the current column widths... should be tested and adjusted.
-
Could this be grouped? Like now is done for base, some thing like:
Base A - Type A
Foo1
Foo2
Base A - Type B
Bar1
Bar2
Base B - Type A
Foo3
-
Please 1 PR = 1 commit
10 commits with random descriptions is really not acceptable.
Roger that. I'll make a new branch with one commit in it.
Also it says WIP, please PR only after it is considered finished.
The WIP is from the fact that it's from the OXCE Suggestions WIP forum.
As for adding prison type, I don't think it will fit all prisoner names and prison names with the current column widths... should be tested and adjusted.
I'll double check.... again. I'll probably use Yanke's suggestion.
-
Could this be grouped? Like now is done for base, some thing like:
Base A - Type A
Foo1
Foo2
Base A - Type B
Bar1
Bar2
Base B - Type A
Foo3
Very doable. I'll give it shot.
-
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)
-
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.
-
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.
-
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...
-
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.
-
Alright... second go around. Here's a PR for this feature request: https://github.com/MeridianOXC/OpenXcom/pull/116
-
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.
-
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.
-
Merged.
Here's the default translations for modders:
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:
STR_PRISON_TYPE_1: "Aquarium"
STR_PRISON_TYPE_2: "Prison"
STR_PRISON_TYPE_3: "Junkyard"
-
Oh wow. Thanks. I was going go back and rework things so you didn't have too.