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

NFSD Criminal Records Fix + Shipyard Fax Records System #1578

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

Qulibly
Copy link
Contributor

@Qulibly Qulibly commented Jun 29, 2024

About the PR

It allows proper function of criminal records by adding a workaround which lets NFSD Outpost receive a record of each player that spawned in no matter in what location, can be Frontier Outpost, Latejoining vessel etc, it will get it without doubling.

This way NFSD Outpost has records of every single player, and criminal record console it is possible to write notes on player records as well as set their wanted level which works globally(from testing)

Adds system for Pirates and future antags. Where it can either fake the records, AKA real job won't be visible and replaced with one of the three most common jobs (contractor, pilot, mercenary), OR where they won't be registered in record at all.

On top of it is added the Shipyard Records, which every time someone purchases a vessel, a special fax will receive information and print an unique paper describing the vessel, captain and details regarding the captain as well as time.
This should drastically aid NFSD in their job as now they can gather and record evidence more easily.
On top of that vessels won't be anonymous anymore, and SR, STC and Sherrif can figure out who owns what ship.

Adds class to each vessel! Which can more precisely describe what kind of hsip and purpose certain vessel is.
The shipyard record paper color will match it's class.

Moreover Blackmarket and Syndicate vessels will have their information mixed with fake one. Giving detectives something to poke at instead of getting all information on silver platter.

=========
IMPORTANT ASPECT
This PR is gonna require mapping extra 2 faxes [vessel records] to Sherrif's and Station Represenative's offices.
Additionally NFSD Outpost will require extra mapped Criminal Records Consoles to be able to access those.

Also vessel Classes will need to be added to every single vessel in the map prototype, otherwise it will use the default "undetermined" type.

Why / Balance

NFSD greatly lacks managing as well as intel gathering tools. This will mainly assist detectives in forensics. Sherrifs in writing more accurate warrants or even sending patrols to check on suspected ships.
Can be used to record infractions on vessel record themselves.
For STC it will be a MASSIVE help as they got essentially list of every single vessel that exists, able to easily write when they docked and when they should undock.
SR will be able to use this information for other functions as well.

How to test

Add "AdditionalStationRecordsComponent" to NFSD Outpost
Latejoin onto station/ship that isnt NFSD outpost after its added
spawn criminal record console on NFSD
check if player information shows up

for shipyard fax records
spawn special fax with "ShipyardRecordPaperComponent" component
go to any shipyard console
purchase a ship
check the paper that this fax outputted

Media

obraz

  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

Any changes to paper and faxes

Changelog

  • fix: Fixed NFSD criminal as well as station records. Now they will contain information of every single play in the sector.
  • tweak: Changed pirate's old records showing in NFSD's criminal records as their old jobs
  • add: Added Shipyard Records Fax system. Which should greatly assist with vessel administration and forensics as each ship has described Captain to them

@dvir001
Copy link
Contributor

dvir001 commented Jun 30, 2024

After upstream update only since this is editing StationRecordsSystem.cs

@Qulibly
Copy link
Contributor Author

Qulibly commented Jun 30, 2024

After upstream update only since this is editing StationRecordsSystem.cs

Thats okay! Plan is to work on finishing the systems here.
Then wait for the upstream merge to be finished.

And fix any resulting issues from the merge.
Will be waiting!

@github-actions github-actions bot added the Merge Conflict This PR has conflicts that prevent merging label Jun 30, 2024
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the Merge Conflict This PR has conflicts that prevent merging label Jul 1, 2024
Copy link
Contributor

github-actions bot commented Jul 7, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the Merge Conflict This PR has conflicts that prevent merging label Jul 7, 2024
@github-actions github-actions bot removed the Merge Conflict This PR has conflicts that prevent merging label Jul 13, 2024
Copy link
Contributor

github-actions bot commented Jul 14, 2024

RSI Diff Bot; head commit f72e196 merging into 4291674
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/_NF/Objects/Misc/shipyard_record_fax_paper.rsi

State Old New Status
paper_record_blackmarket Added
paper_record_cargo Added
paper_record_civilian Added
paper_record_engineering Added
paper_record_expedition Added
paper_record_medical Added
paper_record_nfsd Added
paper_record_salvage Added
paper_record_science Added
paper_record_scrap Added
paper_record_service Added
paper_record_syndicate Added
paper_record_undetermined Added
writing Added

Edit: diff updated after f72e196

@Qulibly
Copy link
Contributor Author

Qulibly commented Jul 14, 2024

obraz
Current biggest issue to resolve is this

As it doesnt care about the place, it will also add pirates into NFSD records which....
Isnt best thing for them as they will get hunted quickly

@Qulibly
Copy link
Contributor Author

Qulibly commented Jul 14, 2024

obraz
Otherwise The Shipyard Fax Record is working well

Got resolved for legal ships as well as illegal

on illegal real information will be mixed with fake generated information
Detectives definetly will have something to work with

hmmm I might also perhaps cahnge the paper format abit so it will clearly state
"Error occured during report formation. Real data corrupted with other data"

or something like that

@Qulibly
Copy link
Contributor Author

Qulibly commented Jul 14, 2024

Oh also get sprite to replace the lab NTX header with something more fitting...

@github-actions github-actions bot added the Map-Shuttle Map - Shuttle label Jul 16, 2024
@Qulibly
Copy link
Contributor Author

Qulibly commented Jul 16, 2024

OKAY should be mostly ready up.
Just needs cleanup... alot of cleanup

and then getting some requirements fit and should be ready

@Qulibly Qulibly marked this pull request as ready for review July 16, 2024 21:13
Fix uppercase mistake
Copy link
Contributor

@whatston3 whatston3 left a comment

Choose a reason for hiding this comment

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

Summary: I like it, not a huge fan of how it's done here - criminal records should use a global record service (see pirate bounties), and shipyard should probably be through a PDA app. Will help when I can on these.

The fundamental idea here is good. I think the idea of having a sector-wide criminal records system makes sense. I don't think the changes as you wrote them do that exactly - I can help get a prototype for that when I have some time.

I'm pushing for a change in the pirate bounties to how sector-wide services can be globally created, accessed and stored (so you only have one "canonical" set of components) rather than using a component query.

I think that having the shipyard records stored in a PDA app might make sense. You won't need to spawn hundreds of entities for ships that will be eventually sold, and you will be able to track ships when they are sold. I know that this requires a lot of rework, happy to help with this when I have time.

I think that using false information for pirates is interesting in theory, but I maintain that the information should be plausible, and appear at first glance as a credible shipyard report.

The class per-shuttle is useful, but you've applied it to very few actual vessels.

The code review I left is largely irrelevant, as I think this needs a good few changes.

Comment on lines +40 to +41
[DataField("class")]
public string Class = "Undetermined";
Copy link
Contributor

Choose a reason for hiding this comment

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

If you move this to an enumeration, you get some "free" validation when creating a new ship.

Comment on lines +264 to +270
var faxQuery = EntityQueryEnumerator<ShipyardRecordPaperComponent>();

while (faxQuery.MoveNext(out var faxUid, out var recordPaperComp))
{
var ev = new ShipyardRecordPaperTransmitEvent(name, _profile.Name, _profile.Species, _profile.Gender, _profile.Age, _fingerprintComponent!.Fingerprint!, _dnaComponent!.DNA!, vessel.Category, vessel.Class, vessel.Group, vessel.Price, vessel.Description);
RaiseLocalEvent(faxUid, ref ev);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does a broadcast event work here instead?

Suggested change
var faxQuery = EntityQueryEnumerator<ShipyardRecordPaperComponent>();
while (faxQuery.MoveNext(out var faxUid, out var recordPaperComp))
{
var ev = new ShipyardRecordPaperTransmitEvent(name, _profile.Name, _profile.Species, _profile.Gender, _profile.Age, _fingerprintComponent!.Fingerprint!, _dnaComponent!.DNA!, vessel.Category, vessel.Class, vessel.Group, vessel.Price, vessel.Description);
RaiseLocalEvent(faxUid, ref ev);
}
RaiseLocalEvent(ref ev);

Comment on lines +55 to +76

/*var query = EntityQueryEnumerator<SectorStationRecordComponent>();

while (query.MoveNext(out var stationGridUid, out var comp))
{
if (TryComp<StationMemberComponent>(stationGridUid, out var stationMemberComponent))
{
var stationEntityUid = stationMemberComponent.Station;

CreateGeneralRecord(stationEntityUid, args.Mob, args.Profile, args.JobId, stationRecords);

/*
if (TryComp<StationRecordKeyStorageComponent>(targetId, out var keyStorage)
&& stationEntityUid != null
&& keyStorage.Key != null)
{
if (!TryGetRecord<GeneralStationRecord>(Key.Value, out var record))
continue;
}
}

}*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this here for a reason?

Suggested change
/*var query = EntityQueryEnumerator<SectorStationRecordComponent>();
while (query.MoveNext(out var stationGridUid, out var comp))
{
if (TryComp<StationMemberComponent>(stationGridUid, out var stationMemberComponent))
{
var stationEntityUid = stationMemberComponent.Station;
CreateGeneralRecord(stationEntityUid, args.Mob, args.Profile, args.JobId, stationRecords);
/*
if (TryComp<StationRecordKeyStorageComponent>(targetId, out var keyStorage)
&& stationEntityUid != null
&& keyStorage.Key != null)
{
if (!TryGetRecord<GeneralStationRecord>(Key.Value, out var record))
continue;
}
}
}*/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With it I believe initially reason for leaving it was to make it easier to in future to upstream merges.
As the original code wiukd ecist as commwnted and in case new changes broke whole aystwm then changes could be disabled if needed.

Though there is chance that the idea behind the commented out block has changed

Comment on lines +123 to +140
// Randomises job
var random = _robustRandom.Next(1, 3);

switch(random)
{
case 1:
playerJob = "Contractor";
break;
case 2:
playerJob = "Pilot";
break;
case 3:
playerJob = "Mercenary";
break;
default:
//Do nothing, when real job is visible that means something bad happened
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

image
I don't think Mercenary will ever be selected. Also,

Suggested change
// Randomises job
var random = _robustRandom.Next(1, 3);
switch(random)
{
case 1:
playerJob = "Contractor";
break;
case 2:
playerJob = "Pilot";
break;
case 3:
playerJob = "Mercenary";
break;
default:
//Do nothing, when real job is visible that means something bad happened
break;
}
string[] jobs = { "Contractor", "Pilot", "Mercenary" };
// Randomises job
var random = _robustRandom.Next(jobs.Length);
playerJob = jobs[random];

Comment on lines +103 to +115
while (stationList.MoveNext(out var stationUid, out var stationRecComp))
{
if (TryComp<StationRecordKeyStorageComponent>(idUid.Value, out var keyStorage)
&& stationEntityUid != null
&& keyStorage.Key != null)
{
if (!TryGetRecord<GeneralStationRecord>(keyStorage.Key.Value, out var record))
continue;

AddRecordEntry((EntityUid) stationEntityUid, record);
break;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this loop? Neither stationUid and stationRecComp are used. Why would the global/sector record set need two separate station records, one for each station?

Might be missing something, but I don't quite get it.

If you want all of the criminal records computers to work on a global database, I think this needs a few changes in order to work, and at that point the access to the computer should probably be locked behind access groups.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have utilised code from the Shipyard that created new record.

Basically wanted to allow another station [NFSD Outpost in current state] to posses these records WITHOUT messing up other aspects like latejoining.

As the current station crew records are very one station heavy at moment codewise

Comment on lines +99 to +100
if (!TryComp<IgnoreSectorStationRecordComponent>(player, out var playerComp))
{
Copy link
Contributor

Choose a reason for hiding this comment

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

If statement avoids the entire while body. Push to the outside.

@Qulibly
Copy link
Contributor Author

Qulibly commented Jul 18, 2024

@whatston3 in regards of ship and class.
I had 0 clue if I should edit it all in THIS PR or leave it for other PRs.
The ones that are currently here are a bleedouver from twsting ans forgot to change them back bwfore comitting.

@whatston3
Copy link
Contributor

Pardon the delay.

@whatston3 in regards of ship and class. I had 0 clue if I should edit it all in THIS PR or leave it for other PRs. The ones that are currently here are a bleedouver from twsting ans forgot to change them back bwfore comitting.

Fair point - I'd probably recommend adding it (merge conflicts be damned, the edits are small enough)

I do want to return to this and help out after pirate bounties, seems like the largest open feature apart from the food rework and cargo vend. A bit of patience 🙏

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the Merge Conflict This PR has conflicts that prevent merging label Jul 24, 2024
@github-actions github-actions bot removed the Merge Conflict This PR has conflicts that prevent merging label Jul 25, 2024
@github-actions github-actions bot added the Merge Conflict This PR has conflicts that prevent merging label Aug 7, 2024
Copy link
Contributor

github-actions bot commented Aug 7, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added Merge Conflict This PR has conflicts that prevent merging and removed Merge Conflict This PR has conflicts that prevent merging labels Aug 12, 2024
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# FTL Map-Shuttle Map - Shuttle Merge Conflict This PR has conflicts that prevent merging Sprites YML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants