Skip to content

Conversation

@drbergman
Copy link
Owner

No description provided.

- signal_exists checks if the given string is a key in all_signals
- behavior_exists similarly for behaviors
- remove signal_scales. they're not used
- no longer need integers to index signals/behaviors
@drbergman drbergman requested a review from Copilot August 23, 2025 23:54
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves signal and behavior handling in the PhysiCell codebase by refactoring the core API design and consolidating repetitive necrotic cell detection logic.

  • Refactors signal and behavior dictionaries to use modern C++ patterns with function objects and improved API design
  • Replaces repetitive necrotic cell detection code with a centralized is_necrotic() method
  • Updates function names and return types for better consistency and type safety

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
sample_projects/celltypes3/custom_modules/custom.cpp Replace necrotic phase checks with is_necrotic() method calls
sample_projects/cancer_immune/custom_modules/cancer_immune_3D.cpp Replace necrotic phase checks with is_necrotic() method calls
modules/PhysiCell_various_outputs.cpp Replace necrotic phase checks and fix loop variable type
modules/PhysiCell_pathology.cpp Replace multiple necrotic phase checks with is_necrotic() method calls
modules/PhysiCell_geometry.cpp Update function calls from find_behavior_index to behavior_exists
core/PhysiCell_signal_behavior.h Major API refactor with new Signal/Behavior structs and function signatures
core/PhysiCell_rules.cpp Update function calls and fix output stream usage
core/PhysiCell_phenotype.h Add const version of current_phase() method
core/PhysiCell_phenotype.cpp Implement const version of current_phase() method
core/PhysiCell_cell.h Add is_necrotic() method declaration
core/PhysiCell_cell.cpp Implement is_necrotic() method with centralized logic

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copilot

This comment was marked as outdated.

@drbergman drbergman requested a review from Copilot August 24, 2025 00:10
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the PhysiCell signal and behavior system to improve code organization and maintainability by replacing manual necrotic state checks with a centralized is_necrotic() method and modernizing the signal/behavior dictionary implementation.

  • Replaces repetitive necrotic phase code checks with a new Cell::is_necrotic() method
  • Modernizes the signal/behavior dictionary system using function objects and improved APIs
  • Updates function calls to use new behavior/signal existence checking methods

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
sample_projects/celltypes3/custom_modules/custom.cpp Uses new is_necrotic() method in coloring functions
sample_projects/cancer_immune/custom_modules/cancer_immune_3D.cpp Uses new is_necrotic() method in coloring function
modules/PhysiCell_various_outputs.cpp Uses new is_necrotic() method and fixes loop variable type
modules/PhysiCell_pathology.cpp Uses new is_necrotic() method across multiple coloring functions
modules/PhysiCell_geometry.cpp Updates to use new behavior existence checking API
core/PhysiCell_signal_behavior.h Complete refactor of signal/behavior system with modern C++ features
core/PhysiCell_rules.cpp Updates to use new signal/behavior existence checking methods and fixes output stream
core/PhysiCell_phenotype.h Adds const version of current_phase() method
core/PhysiCell_phenotype.cpp Implements const version of current_phase() method
core/PhysiCell_cell.h Declares new is_necrotic() method
core/PhysiCell_cell.cpp Implements new is_necrotic() method

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 533 to 535
const Phase& Cycle::current_phase() const
{
return const_cast<Cycle*>(this)->current_phase();
Copy link

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

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

Using const_cast to remove constness and call the non-const version is an anti-pattern. Instead, implement the const version properly by directly accessing the data member, or refactor the non-const version to call the const version."

Suggested change
const Phase& Cycle::current_phase() const
{
return const_cast<Cycle*>(this)->current_phase();
return data.current_phase();

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@drbergman drbergman requested a review from Copilot August 25, 2025 00:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@drbergman drbergman requested a review from Copilot August 25, 2025 09:19
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant