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

Add dirty flag and integrate #1725

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

dmwever
Copy link

@dmwever dmwever commented Dec 4, 2024

fixes #1679. First time working with c++. Been struggling to update a test and getting it to build. Any advice will be helpful. I am using Visual Studio Code.

Please offer much critique. Complain with vigor. Correct with fervor.

@dmwever
Copy link
Author

dmwever commented Dec 4, 2024

A few questions I have for this:
First, how to I integrate the clock with the Map/Terrain/Pathfinding subsystem? I don't just want to start passing the clock through everything, but it seems like, with this task, as well as the Cost Stamp and Record History issues, time will be needed in the pathfinding system. How will this be integrated?

Also, would it be good practice to include non-time based overrides for CostField set_cost which simply set the time to TIME_ZERO for initialization? I have done this for the CostField function that is used by the Map class, but only to avoid needing to pass in a time_t from the map.

Lastly, how do I run with debugging using Visual Studio Code? I make changes to a file and run, but it ignores my breakpoints.

libopenage/pathfinding/cost_field.cpp Outdated Show resolved Hide resolved
libopenage/pathfinding/cost_field.cpp Outdated Show resolved Hide resolved
libopenage/pathfinding/cost_field.cpp Outdated Show resolved Hide resolved
libopenage/pathfinding/cost_field.cpp Outdated Show resolved Hide resolved
libopenage/pathfinding/cost_field.h Outdated Show resolved Hide resolved
libopenage/pathfinding/path.h Outdated Show resolved Hide resolved
libopenage/pathfinding/integrator.h Outdated Show resolved Hide resolved
libopenage/pathfinding/integrator.h Outdated Show resolved Hide resolved
libopenage/pathfinding/sector.h Outdated Show resolved Hide resolved
libopenage/pathfinding/sector.h Outdated Show resolved Hide resolved
@heinezen
Copy link
Member

heinezen commented Dec 5, 2024

First, how to I integrate the clock with the Map/Terrain/Pathfinding subsystem? I don't just want to start passing the clock through everything, but it seems like, with this task, as well as the Cost Stamp and Record History issues, time will be needed in the pathfinding system. How will this be integrated?

Where would you need to pass the clock? You very likely only need to pass the time value but the clock is not necessary IMO.

Also, would it be good practice to include non-time based overrides for CostField set_cost which simply set the time to TIME_ZERO for initialization? I have done this for the CostField function that is used by the Map class, but only to avoid needing to pass in a time_t from the map.

That would kind of counteract introducing dirty flags, so I'm not sure if this is a good idea. In fact, I'm not really sure why we should just always pass a time. Initialization does not always happen at time zero.

Lastly, how do I run with debugging using Visual Studio Code? I make changes to a file and run, but it ignores my breakpoints.

It should just work out of the box. What is your start configuration and output?

@dmwever
Copy link
Author

dmwever commented Dec 6, 2024

First, how to I integrate the clock with the Map/Terrain/Pathfinding subsystem? I don't just want to start passing the clock through everything, but it seems like, with this task, as well as the Cost Stamp and Record History issues, time will be needed in the pathfinding system. How will this be integrated?

Where would you need to pass the clock? You very likely only need to pass the time value but the clock is not necessary IMO.

Perhaps that's a better question. How to get the time (or any time) into the Map subsystem.

dmwever and others added 4 commits December 5, 2024 22:56
Co-authored-by: Christoph Heine <6852422+heinezen@users.noreply.github.com>
Co-authored-by: Christoph Heine <6852422+heinezen@users.noreply.github.com>
Co-authored-by: Christoph Heine <6852422+heinezen@users.noreply.github.com>
@heinezen
Copy link
Member

heinezen commented Dec 7, 2024

Perhaps that's a better question. How to get the time (or any time) into the Map subsystem.

The update time comes from events in the event system that will trigger a map update. However, I wouldn't worry about that right now, since it is not implemented yet.

@dmwever
Copy link
Author

dmwever commented Dec 7, 2024

It should just work out of the box. What is your start configuration and output?

If you are talking about the configurations in launch.json, it says:
"configurations": []

That's probably my problem lol. What should it say?

@heinezen
Copy link
Member

heinezen commented Dec 7, 2024

Something like this should work as a configuration:

{
    "name": "Main",
    "type": "cppdbg",
    "request": "launch",
    "cwd": "${workspaceFolder}/bin",
    "program": "${workspaceFolder}/bin/run",
    "args": [
        "main",
        "--verbose",
        "--gl-debug",
    ],
    "setupCommands": [
        {
            "description": "Enable pretty-printing for gdb",
            "text": "-enable-pretty-printing",
            "ignoreFailures": true
        },
        {
            "text": "source ${workspaceFolder}/etc/openage.gdbinit"
        }
    ],
}

@dmwever
Copy link
Author

dmwever commented Dec 7, 2024

Something like this should work as a configuration:

{
    "name": "Main",
    "type": "cppdbg",
    "request": "launch",
    "cwd": "${workspaceFolder}/bin",
    "program": "${workspaceFolder}/bin/run",
    "args": [
        "main",
        "--verbose",
        "--gl-debug",
    ],
    "setupCommands": [
        {
            "description": "Enable pretty-printing for gdb",
            "text": "-enable-pretty-printing",
            "ignoreFailures": true
        },
        {
            "text": "source ${workspaceFolder}/etc/openage.gdbinit"
        }
    ],
}

So to clarify, I should put this in launch.json? I am used to Visual Studio with C#.

@heinezen
Copy link
Member

heinezen commented Dec 7, 2024

You should put it in the configurations array, yes.

@dmwever
Copy link
Author

dmwever commented Dec 7, 2024

You should put it in the configurations array, yes.

[workspace]/bin/run does not exist

@heinezen
Copy link
Member

heinezen commented Dec 7, 2024

Well, you need to build the project before running :D

@heinezen
Copy link
Member

heinezen commented Dec 7, 2024

https://github.com/SFTtech/openage/blob/master/doc/building.md has info on the build procedure.

libopenage/pathfinding/cost_field.h Outdated Show resolved Hide resolved
libopenage/pathfinding/cost_field.h Outdated Show resolved Hide resolved
libopenage/pathfinding/integrator.cpp Outdated Show resolved Hide resolved
Comment on lines 111 to 113
if (evict_cache) {
this->field_cache->evict(cache_key);
}
Copy link
Member

Choose a reason for hiding this comment

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

same here

Copy link
Author

Choose a reason for hiding this comment

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

The integrate function does not pass in the cost field. I am going to remove the dirty check from the integrate function for now, unless we want to add dirty flags to the integration fields as well.

Copy link
Member

@heinezen heinezen Dec 24, 2024

Choose a reason for hiding this comment

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

integrate receives const std::shared_ptr<CostField> &cost_field as its first argument, so you should be able to use it for checking whether the field is dirty.

Copy link
Author

@dmwever dmwever Dec 25, 2024

Choose a reason for hiding this comment

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

sorry, I meant build

@heinezen heinezen added improvement Enhancement of an existing component lang: c++ Done in C++ code area: simulation Involved in the game mechanics and simulation labels Dec 15, 2024
@dmwever
Copy link
Author

dmwever commented Dec 21, 2024

https://github.com/SFTtech/openage/blob/master/doc/building.md has info on the build procedure.

I can't seem to get this process working on windows. I can run using python openage, following the info on this page:
https://github.com/SFTtech/openage/blob/master/doc/build_instructions/windows_msvc.md

But I don't see a bin folder built in openage following the Windows instructions.

@jere8184
Copy link
Contributor

jere8184 commented Dec 21, 2024

https://github.com/SFTtech/openage/blob/master/doc/building.md has info on the build procedure.

I can't seem to get this process working on windows. I can run using python openage, following the info on this page: https://github.com/SFTtech/openage/blob/master/doc/build_instructions/windows_msvc.md

But I don't see a bin folder built in openage following the Windows instructions.

I can walk you through it at matrix if you want

libopenage/pathfinding/cost_field.h Outdated Show resolved Hide resolved
Comment on lines 90 to 93
inline void set_cost(size_t idx, cost_t cost, const time::time_t &until) {
cells[idx] = cost;
valid_until = until;
}
Copy link
Member

Choose a reason for hiding this comment

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

This method should be documented.

libopenage/pathfinding/cost_field.h Outdated Show resolved Hide resolved
libopenage/pathfinding/cost_field.h Outdated Show resolved Hide resolved
dmwever and others added 2 commits December 24, 2024 20:48
Co-authored-by: Christoph Heine <6852422+heinezen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: simulation Involved in the game mechanics and simulation improvement Enhancement of an existing component lang: c++ Done in C++ code
Projects
Status: 📋 Backlog
Development

Successfully merging this pull request may close these issues.

Invalidate flow field cache entry when cost field changes
3 participants