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

feat: fishing skill #390

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions data/config/npc-spawns/lumbridge/lumbridge-fishing.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"npc": "rs:fishing_spot_bait_a",
"spawn_x": 3239,
"spawn_y": 3241,
"spawn_level": 0,
"movement_radius": 0,
"face": "WEST"
},
{
"npc": "rs:fishing_spot_bait_a",
"spawn_x": 3239,
"spawn_y": 3244,
"spawn_level": 0,
"movement_radius": 0,
"face": "WEST"
}
]
5 changes: 5 additions & 0 deletions data/config/npcs/fishing.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rs:fishing_spot_bait_a": {
"game_id": 233
}
}
100 changes: 100 additions & 0 deletions src/engine/task/impl/actor-actor-interaction-task.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { LandscapeObject } from '@runejs/filestore';
import { activeWorld, Position } from '@engine/world';
import { Actor } from '@engine/world/actor';
import { ActorWalkToTask } from './actor-walk-to-task';

/**
* A task for an {@link Actor} to interact with another {@link Actor}.
*
* This task extends {@link ActorWalkToTask} and will walk the actor to the object.
* Once the actor is within range of the target Actor, the task will expose the `otherActor` property
*
* @author jameshallam
*/
export abstract class ActorActorInteractionTask<TActor extends Actor = Actor, TOtherActor extends Actor = Actor> extends ActorWalkToTask<TActor, Position> {
/*
* TODO (jameskmonger) consider exposing this, currently people must always access it through `otherActor`
* or through their own constructor
*/
private _targetActor: TOtherActor;

/**
* Gets the {@link TOtherActor} that this task is interacting with.
*
* @returns The target actor, if is still present, and if the actor is at the destination.
* Otherwise, `null`.
*
* TODO (jameskmonger) unit test this
*/
protected get otherActor(): TOtherActor | null {
if (!this.atDestination) {
return null;
}

if (!this._targetActor) {
return null;
}

return this._targetActor;
}

/**
* Get the position of this task's target npc
*
* @returns The position of this task's target npc, or `null` if the npc is not present
*/
protected get otherActorPosition(): Position {
if (!this._targetActor) {
return null;
}
return this._targetActor.position
}

/**
* @param actor The actor executing this task.
* @param targetActor The `TOtherActor` to interact with.
* @param sizeX The size of the target TOtherActor in the X direction.
* @param sizeY The size of the target TOtherActor in the Y direction.
*/
constructor (
actor: TActor,
targetActor: TOtherActor,
sizeX: number = 1,
sizeY: number = 1
) {
super(
actor,
// TODO (jameskmonger) this doesn't currently account for a moving NPC target
targetActor.position,
Comment on lines +67 to +68
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should change ActorWalkToTask to also accept Actor rather than passing the position in manually, that way we can easily defer the "tracking" of moving targets easily.

Copy link
Collaborator

Choose a reason for hiding this comment

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

then this would just be

Suggested change
// TODO (jameskmonger) this doesn't currently account for a moving NPC target
targetActor.position,
targetActor,

Math.max(sizeX, sizeY)
);

if (!targetActor) {
this.stop();
return;
}

this._targetActor = targetActor;
}

/**
* Checks for the continued presence of the target {@link Actor}, and stops the task if it is no longer present.
*
* TODO (jameskmonger) unit test this
*/
public execute() {
super.execute();

if (!this.isActive || !this.atDestination) {
return;
}

// stop the task if the actor no longer exists
if (!this._targetActor) {
this.stop();
return;
}

// TODO: check npc still exists
Copy link
Collaborator

Choose a reason for hiding this comment

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

need to investigate what happens when NPCs die/are removed

I suspect we need to do something similar to the activeWorld.findObjectAtLocation in ActorLandscapeObjectInteractionTask

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import { ActorWalkToTask } from './actor-walk-to-task';
* @author jameskmonger
*/
export abstract class ActorLandscapeObjectInteractionTask<TActor extends Actor = Actor> extends ActorWalkToTask<TActor, LandscapeObject> {
/*
* TODO (jameskmonger) consider exposing this, currently people must always access it through `otherActor`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* TODO (jameskmonger) consider exposing this, currently people must always access it through `otherActor`
* TODO (jameskmonger) consider exposing this, currently people must always access it through `landscapeObject`

my bad

* or through their own constructor
*/
private _landscapeObject: LandscapeObject;
private _objectPosition: Position;

Expand All @@ -24,9 +28,6 @@ export abstract class ActorLandscapeObjectInteractionTask<TActor extends Actor =
* TODO (jameskmonger) unit test this
*/
protected get landscapeObject(): LandscapeObject | null {
// TODO (jameskmonger) consider if we want to do these checks rather than delegating to the child task
// as currently the subclass has to store it in a subclass property if it wants to use it
// without these checks
if (!this.atDestination) {
return null;
}
Expand Down Expand Up @@ -91,11 +92,13 @@ export abstract class ActorLandscapeObjectInteractionTask<TActor extends Actor =
return;
}

// stop the task if the object no longer exists
if (!this._landscapeObject) {
this.stop();
return;
}

// find the object in the world and validate that it still exists
const { object: worldObject } = activeWorld.findObjectAtLocation(this.actor, this._landscapeObject.objectId, this._objectPosition);

if (!worldObject) {
Expand Down
1 change: 1 addition & 0 deletions src/engine/task/impl/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export { ActorTask } from './actor-task'
export { ActorWalkToTask } from './actor-walk-to-task'
export { ActorWorldItemInteractionTask } from './actor-world-item-interaction-task'
export { ActorLandscapeObjectInteractionTask } from './actor-landscape-object-interaction-task'
export { ActorActorInteractionTask } from './actor-actor-interaction-task'
22 changes: 22 additions & 0 deletions src/engine/world/config/harvest-tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,27 @@ const Axes: HarvestTool[] = [
{ itemId: 6739, level: 61, animation: 2846 }
];

const FishingRods: HarvestTool[] = [
{ itemId: 309, level: 1, animation: 1363 }
];

/**
* Checks the players inventory and equipment for fishing rod
* @param player
* @return the highest level pickage the player can use, or null if theres none found
*/
export function getFishingRod(player: Player): HarvestTool | null {
for (let i = FishingRods.length - 1; i >= 0; i--) {
if (player.skills.hasLevel(Skill.FISHING, FishingRods[i].level)) {
if (player.hasItemOnPerson(FishingRods[i].itemId)) {
return FishingRods[i];
}
}
}
return null;
}


/**
* Checks the players inventory and equipment for pickaxe
* @param player
Expand All @@ -63,6 +84,7 @@ export function getBestPickaxe(player: Player): HarvestTool | null {
}
return null;
}

/**
* Checks the players inventory and equipment for axe
* @param player
Expand Down
33 changes: 28 additions & 5 deletions src/engine/world/skill-util/harvest-skill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Player } from '@engine/world/actor/player/player';
import { IHarvestable } from '@engine/world/config/harvestable-object';
import { soundIds } from '@engine/world/config/sound-ids';
import { Skill } from '@engine/world/actor/skills';
import { getBestAxe, getBestPickaxe, HarvestTool } from '@engine/world/config/harvest-tool';
import { getBestAxe, getBestPickaxe, HarvestTool, getFishingRod } from '@engine/world/config/harvest-tool';
import { randomBetween } from '@engine/util/num';
import { ObjectInteractionAction } from '@engine/action';
import { colors } from '@engine/util/colors';
Expand All @@ -18,19 +18,20 @@ import { loopingEvent } from '@engine/plugins';
*
* @returns a {@link HarvestTool} if the player can harvest the object, or undefined if they cannot.
*/
export function canInitiateHarvest(player: Player, target: IHarvestable, skill: Skill): undefined | HarvestTool {
export function canInitiateHarvest(player: Player, target: Pick<IHarvestable, 'itemId' | 'level'>, skill: Skill): undefined | HarvestTool {
if (!target) {
switch (skill) {
case Skill.MINING:
player.sendMessage('There is current no ore available in this rock.');
player.playSound(soundIds.oreEmpty, 7, 0);
break;
case Skill.FISHING:
player.sendMessage('There are no fish in that spot.');
break;
default:
player.sendMessage(colorText('HARVEST SKILL ERROR, PLEASE CONTACT DEVELOPERS', colors.red));
break;


}
player.playSound(soundIds.oreEmpty, 7, 0);
return;
}

Expand All @@ -39,6 +40,9 @@ export function canInitiateHarvest(player: Player, target: IHarvestable, skill:
case Skill.MINING:
targetName = targetName.replace(' ore', '');
break;
case Skill.FISHING:
targetName = 'fish';
break;
}


Expand All @@ -48,27 +52,41 @@ export function canInitiateHarvest(player: Player, target: IHarvestable, skill:
case Skill.MINING:
player.sendMessage(`You need a Mining level of ${target.level} to mine this rock.`, true);
break;
case Skill.FISHING:
player.sendMessage(`You need a Fishing level of ${target.level} to fish at this spot.`);
break;
case Skill.WOODCUTTING:
player.sendMessage(`You need a Woodcutting level of ${target.level} to chop down this tree.`, true);
break;
}
return;
}

// Check the players equipment and inventory for a tool
let tool;
switch (skill) {
case Skill.MINING:
tool = getBestPickaxe(player);
break;
case Skill.FISHING:
// TODO (jameshallam93): different spots need different equipment
tool = getFishingRod(player);
break;
case Skill.WOODCUTTING:
tool = getBestAxe(player);
break;
}

// TODO (jameshallam93): some activities need more than one tool, e.g. bait

if (tool == null) {
switch (skill) {
case Skill.MINING:
player.sendMessage('You do not have a pickaxe for which you have the level to use.');
break;
case Skill.FISHING:
player.sendMessage('You do not have a fishing rod for which you have the level to use.');
break;
case Skill.WOODCUTTING:
player.sendMessage('You do not have an axe for which you have the level to use.');
break;
Expand All @@ -88,9 +106,11 @@ export function canInitiateHarvest(player: Player, target: IHarvestable, skill:

export function handleHarvesting(details: ObjectInteractionAction, tool: HarvestTool, target: IHarvestable, skill: Skill): void {
let itemToAdd = target.itemId;
// This is rune essence to pure essence
if (itemToAdd === 1436 && details.player.skills.hasLevel(Skill.MINING, 30)) {
itemToAdd = 7936;
}
// This is to deal with gem rocks
if (details.object.objectId === 2111 && details.player.skills.hasLevel(Skill.MINING, 30)) {
itemToAdd = rollGemRockResult().itemId;
}
Expand All @@ -106,6 +126,9 @@ export function handleHarvesting(details: ObjectInteractionAction, tool: Harvest
case Skill.MINING:
details.player.sendMessage('You swing your pick at the rock.');
break;
case Skill.FISHING:
details.player.sendMessage('You cast your line out.');
break;
case Skill.WOODCUTTING:
details.player.sendMessage('You swing your axe at the tree.');
break;
Expand Down
Loading