BibliothecaDAO / realms-contracts

Realms Monorepo for Ethereum contracts and Starknet contracts.
https://bibliothecadao.xyz/
MIT License
87 stars 33 forks source link

change travel time fn / add arrival time fn #231

Closed d-s-i closed 1 year ago

d-s-i commented 2 years ago

Current Travel.get_travel_time function:

@view
func get_arrival_time{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
    traveller_coordinates: Point, destination_coordinates: Point
) -> (time: felt) {
    // get distance between two points
    let (distance) = get_travel_distance(traveller_coordinates, destination_coordinates);

    // calculate time
    let (time) = Travel.calculate_time(distance);

    let (now) = get_block_timestamp();

    return (time + now,);
}

New get_travel_time function:


@view
func get_travel_time{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
    traveller_coordinates: Point, destination_coordinates: Point
) -> (time: felt) {
    // get distance between two points
    let (distance) = get_travel_distance(traveller_coordinates, destination_coordinates);

    // calculate time
    let (time) = Travel.calculate_time(distance);

    return (time,);
}

New get_arrival_time function:

@view
func get_arrival_time{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
    traveller_coordinates: Point, destination_coordinates: Point
) -> (time: felt) {
    // get distance between two points
    let (distance) = get_travel_distance(traveller_coordinates, destination_coordinates);

    // calculate time
    let (time) = Travel.calculate_time(distance);

    let (now) = get_block_timestamp();

    return (time + now,);
}

This is taking the travel time and adding the current timestamp, returning the timestamp at arrival. I think this is more appropriate to call this get_arrival_time returning the timestamp at arrival and creating a function get_travel_time simply returning the time of the travel.

This is not very important, but I think this function maybe be used a lot and can be confusing. Hope small PRs like those are welcomed

ponderingdemocritus commented 2 years ago

yes, good idea. This is clearer. Could you update in the travel function