search

TeamPorcupine / ProjectPorcupine

Project Porcupine: A Base-Building Game...in Space!
GNU General Public License v3.0
484 stars 279 forks source link

New Job System #1590

Open BraedonWooding opened 7 years ago

BraedonWooding commented 7 years ago

Outline

Okay so essentially the Job System is broken, it really needs a full re-write; and well I'm trying to do it so it'll be so perfect it'll never need another re-write/cleanup (aim for the moon so you land among the stars).

I know this is going to be a pain for other PRs, but the longer we wait on this the worse it'll get, so I'm pulling the band-aid off (so to speak).

Also I'm not going to create a PR till its testable (there will be a checklist at the end to indicate progress before PR). Also this issue took like 2 solid hours to build so be kind if like typos or doesn't fit format or whatever 😩.

Initial Progress

So I'm already working on the system (super early work just getting class structures in place) and I'm building this issue so we can decide on direction and little things as well as big things.

Also I'm suffixing most classes with "" just so I don't have weird conflicts (like two Job classes) and I can slowly move parts across, then i'll do a find and replace (like replace "Job" with "Job") the double underscore makes sure that it isn't including xml or json or any text / property (which can use sometimes a single underscore).

The core implementation

Why Interfaces and Abstract classes?

I'm addicted to interfaces and abstract classes (a little too much perhaps), but I think it fits this situation perfectly; below I'll place a few pictures of my proposed interface hierarchy for the Job System.

NOTE: The layers represent inheritance (the program broke and I couldn't draw the lines), all jobs inherit from the interface above them AND BaseJob untitled

Now as you can see it's quite extensive and VERY powerful (you could move the current state system (or other systems) to using these exact interfaces if you wanted to), so this opens up the modability a ton! So basically interfaces and abstract classes require good structure (so no re-write in future) and open doors fully to modability and nice polymorphism (casting down/up from x to y).

JobManager

Essentially what we are doing is building a manager that manages (lol) the current jobs by assigning specific task lists to characters based upon proximity (a radius from a tile based upon that character's current list of jobs), and preference.

Now why is this important?

Speed is the main reason, we can thread these processes and essentially do some early pathfinding to speed up the actual pathfinding (like using @koosemose current room pathfinding where it locates room -> room), then when the character gets the task it already knows what rooms it needs to pass and then can just do a finer optimised path (doesn't even need to find the most optimal just within 95% of the optimal path - one square slower for every 20).

Priorities

The priority system is number based, but to also have critical priorities and based off how normally managers work I have decided to implement it so the only value that matters is the absolute value of the number (its distance from zero), with a negative number indicating that the job list should be rebuilt for the character that should get that negative number. Now this is so that 1/-1 is the highest priority, 2 being the second highest, 3 being the third highest (and 0 being just add it to the end or where-ever), if you add a job of priority 2 for example, the next time the job manager is giving a list of jobs to the character it will put the job second (well like not 'second' cause there may be multiple priority one jobs but essentially after all the priority one jobs, also if there are multiple second priority jobs then which one goes where is entirely dependent on the list creation and there is no way to say 1.5 or 1.9 for example. However if you put -2 then after it calculates the most optimal character to do that job (or if you specify a character) it will grab that character and say refresh your list, the character will be given a new list of jobs with -2 being placed second in that list (with the same rules as if it was just normal 2, except it will be placed before all other second priority jobs).

If you have any questions about this (or anything at all) just pop it below, if there is still conjuncture (no definite answer) then it'll be put in the question table :D.

Modding

Now we have some standard job 'types' (classes) that will be used mainly (stuff like haul, build object, build furniture, operate, produce object, perform on furniture/tile) but these can be subclasses for functionality, as well as whole new types can be built (stuff like a hunt job could be super easy to implement) this means that later on someone could build whole new jobs (as long as they use the interfaces provided with the game).

Feedback/Questions

I'll populate this as questions pop up but I've got a few to start with

Question Current answer to question Community Accepted Answer
How could/would 'needs' be implemented with said system? You should never use jobs to figure out what to do, a job is saying go do this. So with needs what you could do is let's say you have a sleep need, so the sleep need is at 20 for Bob (with 0 being that you need sleep or dead) so you could say hey JobManager add a job (sleepyJob) with priority 2, with a preference of 0 to Bob (0 meaning that only Bob can have that job); this means that the next time JobManager delegates jobs out to people make this the second highest priority job for Bob! But now the sleep need is at 10 (due to Bob having priority 1 jobs), so you could say okay JobManager change job of type (sleepyJob) for Bob to being priority 1 (you can either pass a reference to that job or you can specify a character, or say change every job that matches this type), now this doesn't refresh Bob's list (because it's still a positive number), so Bob still doesn't get any sleep. Now his need is at 0 so he needs sleep right now, so you set sleepyJob to -1, which means interrupt Bob's current list and pass him a new list with sleepyJob being priority 1 (the highest priority, and since you are passing an interrupting priority - negative - then it will also make sure sleepyJob is the first 1 priority). No accepted answer yet.
Batch Jobs as a flag or a job class Now as the manager plans out jobs it could either detect one of two ways (both valid), first is that it detects a job as being a batch job (a flag that all jobs have) and then it searches for nearby jobs, so any object that COULD be batch has this flag turned on, it just does a quick proximity search (maybe max of 5/7 squares 2 dimensionally) then if similar jobs exist (same type of job or same job or whatever) it then will bunch them together in a new job type that is evaluated as a single job that is passed to a character without refreshing (unless an interruption occurs). The other way is that the build manager in this case would pass this new job type? Well you could do it both ways??? No accepted answer yet.
How jobs are handled/updated The old job system implementation has functions (for LUA and C#) that it calls on start/update/pause... My suggestion is having handlers that get given a IJob (base interface) with a work modifier and handle the updating, now if you wanted an UI updater than you would create an handler and then subscribe to the job's UI Update handler (every half second??). This way you would have generic handlers for Jobs and if you wanted to override it you will have to subclass that Job class but in most cases you won't need to override it at all? Or we could use the old job system implementation No accepted answer yet.
How should the preference system be implemented A weighting system of sorts (a 10 in construction may be lower than being right next to it with all the materials). No accepted answer yet.
(@Geoffrotism) Don't all jobs require a tile? Shouldn't IJobTile be part of IJob The current system is that a Job may 'regard a tile' or may 'regard a furniture' and so on, but the idea of having all jobs 'regard a tile' and some also 'regard a furniture' is also a possibility No accepted answer yet.
(@NogginBops @dusho) How should inheritance work? Two ways either a interface based approach, or a generalised approach Generalised imo (and using the previous job system as an example) in most situations is bad, you're making presumptions on how things will work in the future and what features will be added (which even if you know 100% what's going to happen can be bad for modding), which means in an open source project we need to make sure that our code lasts a long time before it needs to be re-written to allow for more functionality (otherwise every year it's just rewriting the same old stuff to allow for new stuff and you get completely unreadable hunks of mess classes like the current Job class - took me a solid hour or two just to read the 100s of functions), so it's clear we need some kind of inheritance based approach, I personally like interfaces but that's mainly because I'm using them a lot in my work, and other places so I'm a little biased.

Progress before PR can be made/tested

--- This is far enough for testing to happen (PR request made).

Progress till finished

Okay so I thought I really need to add a quick section here talking about the inheritance of the job system.

Okay so i've tiered it, 1) tier one is what we call the baseJob it doesn't inherit from anything and essentially acts as the old IJob & IJobPriority rolled into one small class.
2) Then we have our generic classes which will then have specialised classes as the third tier. This way you may have multiple produceJobs that have different logic (maybe one takes in multiple inputs for multiple outputs and so on).

Geoffrotism commented 7 years ago

the only thing i can think of immediately is the priorities. and this starts getting out there, but how do we want to do them. I know the numbers on priorities could be changed easily, but just in general. How much micromanaging do we want our user to be allowed to have? I think the system from Rimworld is perfect for us but would require just bit more sorting on job assigning. This would mean just a bit more sorting when the job list is created but would add more micro for the user if they so wish. Need to make the default intuitive so that users not wanting to micro that much wont get ruined.

BraedonWooding commented 7 years ago

Well we could use the positive negative thing i suggested; so that the jobManager would search for possible job candidates and when it gives it to the character; it would give the job a priority that the user set (1+) so essentially a carbon copy of rimworld's with support for an infinite priority cap, and well the automatic priorities would just set to 0. And if u right click and prioritise then it would apply -1 priority

koosemose commented 7 years ago

Assuming I'm understanding what you're saying regarding needing tiles correctly, I don't think all jobs should require a tile (or a furniture), with the most notable examples being jobs given by needs: if sleep needs creates a job to go to sleep, when creating the job, the need shouldn't have to know specifically what tile/furniture will be used to sleep, and the job manager in turn shouldn't be what figures out what tile/furniture is needed to sleep, the need would most likely assign a job and would tell it that it needs a certain type of furniture (in this case a bed), job manager would just keep track of that until it passes it on to the character, the character in turn would be responsible for finding the bed (though it may or may not do this using an evaluator function passed from the need), or may eventually have some means of remembering what bed to use and just pathfind to that (something like rimworld's assigned beds), and if it has to search for an unclaimed bed, would presumably claim that one (through some fashion) upon finding it.

In turn an oxygen need doesn't care so much about a specific tile or furniture, but a room with enough oxygen (though this could be coded in such a way that it is looking for a tile with a room with enough oxygen). Of course, optimally, rather than the O2 need job having to just give a job to just stand around for a while in an oxygenated room, it would be nice if after giving it a job to go to an oxygenated room, it could set a limit through some fashion on what jobs a character will take, so a character gets an urgent oxygen need, goes to an oxygenated room immediately, and then only work jobs within that area until their need rises to a certain level and removes that restriction.

Geoffrotism commented 7 years ago

@koosemose sure but you'll need to have an evaluator in JobManager anyway to handle unreachable jobs/ jobs that don't have enough materials around yet. Similar to what we have currently.

BraedonWooding commented 7 years ago

Okay so @koosemose this is what the system is trying to avoid, the character is never to do intense calculations (like finding a bed), in this case what would happen for sleep is: okay you need to sleep; 1) you have a bed allocated to you sleep there; 2) you dont have a bed so get list of furniture of that type that isn't occupied and then get your position and perform basic room heuristics to get about an 80-90% accurate bed (accuracy as in closest). This way we can utilise the threading to a greater extent.

NogginBops commented 7 years ago

I'm looking at the diagram and I have to say, I really don't like it and I really don't think this will benefit PP in any way. Inheritance can be useful when you have like one or two layers but this will just make the whole process a lot more complicated and so much more obfuscated than it needs to be. @vogonistic has been working on replacing the job system for some time now and this has been discussed in a lot of other issues, see: #726 #86 #75 #63.

Really in games (and imo in all applications) you really should strive for as flat of a hierarchy as possible as unnecessary inheritance and abstractions make the code harder to understand and impact performance (if only a little bit).

This also highlights a bigger problem I think we are facing in PP, different programming styles and coding philosophies. In the beginning PP was coded with the idea of minimal inheritance and just making versatile and generic classes. This proposal however is more making very specific classes for very small functionalities (An interface with only one property or method declaration) and a very big inheritance tree. This is something I think we might need to have a discussion about in the future but that is for another issue.

All in all, what this proposal reminds me of is this project. :P Which I think we really wan't to stay away from becoming.

dusho commented 7 years ago

whoa.. you really used lots of classes and interfaces there. I also had something more simple in mind. You're mentioning what modders can do with the system, but I wouldn't be sure modders would actually use all of this without deep knowledge of the code. When you said in chat that you will have something like JobManager.CreateJob(new HaulJob()) , I imagined maybe to have elemental jobs (part of the core - not really exposed for modding) and then CreateJob would consist of series of jobs that would succeed each other (depending on conditions). So modders or anyone could create new jobs just by creating composite job out of elemental ones.. not by actually creating new implementation of interface (or several). Other than that I would love to see it threaded like you're proposing.

BraedonWooding commented 7 years ago

Modders don't require knowledge of the inheritance in 95% of cases they only require knowledge of the specialised classes (haul, produce...), but if they wanted to create a new type of job (like hunt) you need a system like this. Yes you could flatten the hierarchy a little, but its all about abstraction, saying this job has a timer so do this, this job can produce so on. The whole idea that flatter hierarchies are better is only for simple systems, you can't use "generalised classes" with a job system... like thats what the old system used and it didn't work you need to use a form of inheritance. What the interfaces implement (hehe) is allowing me to have components of a job do different things rather than saying hey "is x of type y" i can say "hey does x conform to type y".

Geoffrotism commented 7 years ago

I'm all for a better system, but I'd like to point out the current system does work so far. It needs love for sure but without a clear goal of what micro we want (I.E. stockpile only takes items of type beer and only accepts items being produced from the kitchen. For a case where only the elite staff get the good, local brews and the other staff get crappy imports.) it gets murky.

koosemose commented 7 years ago

Now that I've had time to actually look at your diagram, I have to agree with @NogginBops , this seems way over complicated, and in many cases seems to be using interfaces just for the sake of having interfaces, with the most notable examples being IJob and IJobPriority, why would a job ever not be a job or have a priority? and if so why do these need to be separate interfaces, rather than just part of BaseJob. Why would a job that doesn't take any work time need a special interface (or lack thereof) rather than just having 0 time required, it will complete instantly either way.

And I don't see any reason that things should be kept out of Character just based on some arbitrary determination of "shouldn't be doing intense calculations", if it's something the character is doing then it should be up to Character as to how to do it. This will just end up with a character's AI scattered among I don't know how many different classes in the end.

BraedonWooding commented 7 years ago

Yep I agree, as I did say above 100% it could and parts should be flattened, how I write code is I use interfaces verbose then remove them when I start making classes that implement them so you just kind of ended up with a verbose hierarchy. I've just flattened the inheritance (I didn't add/remove all the properties though since I felt the inheritance was the important part), most inheritance is only 1 or 2 layers deep its just there is some jobs that come from here and there and so on. Geo, this system is trying to do exactly that! Say okay I'm moving 10 beer from a tile (the haul command asks for an item and for an amount as well as an initial and a final - should probably also be a type) to a stockpile, then the manager finds the most appropriate stockpile based on priority and proximity (to the position of the beer) and tada! Now if you wanted a job that is go kill something you could have an operate functionality on a tile, then a haul functionality. Again yep its a little overboard (I even said that somewhere in there) but the IDEA is what the chart was for the idea that you have components of a job.

BraedonWooding commented 7 years ago

Also on the character bit, It's not about removing character code its about optimising it, its about giving a job to a character that is go do this RATHER than saying go figure out how to do this! It doesn't pathfind just figures out the closet via some distance equations that use room sizes and proximity to figure out a rough estimate (my equation sits at around 80-90% accuracy - higher with more walls).

bjubes commented 7 years ago

In your diagram IJobAction is redundant of the base job class. Did you mean to remove it along with the other interfaces @koosemose mentioned?

BraedonWooding commented 7 years ago

Yep I did mean to remove it, I just missed it when I was cleaning up the diagram.

BraedonWooding commented 7 years ago

Note: this from this point onward this issue will be go through a restructure so the comments may not make sense relating to the issue itself.